[configuration] AbstractConfigurationKonstantin, The changes look good. And I checked through the Turbine codebase, and the changes you propose don't look like they will cause problems....
Could you do me a favor and submit a patch file? Not sure what editor you use, but Eclipse does a good job of creating patch files. Also, run the maven site and make sure your changes don't cause more checkstyle errors. You seem to use 2 spaces for a tab versus 4, a couple other little things as well. Although actually the BaseConfiguration needs lots of checkstyle help as it has 194 violations! Don't forget to add yourself to the contributor list in the project.xml and the @author tags;-) I agree with the testBoolean, it should return a boolean value I think as well. As far as the addProperty/addPropertyDirect, I would recommend that you add a unit test to the original code that tests it. then, apply your changes, and verify the tests work the same way. To be honest, I never had to look too closely at that part. So, if you can try and resolve some of those questions, and get the formatting in line with the standard style then I'll be happy to commit these changes. And then we can discuss your JDBC implementation which I am very curious to see... Eric -----Original Message----- From: Konstantin Shaposhnikov [mailto:[EMAIL PROTECTED] Sent: Sunday, September 28, 2003 8:16 PM To: 'Jakarta Commons Developers List' Subject: [configuration] AbstractConfiguration Hello Eric, Pease see attached files: AbstractConfiguration.java - AbstractConfiguration class, actually this is slightly modified BaseConfiguration class (I've addes some methods and made some methods abstract) BaseConfiguration.java - BaseConfiguration class modified to extend Abstract configuration TestBaseConfiguration.java - I add one test method to test new behaviour of getString method and default configuration. All tests are passed succesfully (maven test :) . I also put some my comments in the source code. Please review them. You are talking about getObject method but there is no such method in Configuration interface. May be it will be usefull to add it? As for a JDBCConfiguration class, I actually need very specific to my project configuration implementation (using Hibernate and special logic to access database). But we can discuss this class and may be I'll implement them for commons-configuration project. In any case I think that first we should refactor existing Configuration implementations to use AbstractConfiguration (of course if you accept it). On 17:41 Fri 26 Sep, Eric Pugh wrote: > Konstantin, > > I like your idea about trying to make things easier. I have been wanting to > write a JDBCConfiguration class, but haven't gotten around to it. > > The BaseConfiguration works the way it does I think because it keeps config > values in two seperate lists, correct? One in memory, and one loaded by the > user? > > What if we instead create an AbstractConfiguration class that overrides > everything. Then, when you implement your own, you just override the > methods you want? Similar to the approach taken by java.util.AbstractList? > > If you can supply the code with unit tests, I would be very happy to review > and commit it. > > As far as the NoSuchElementException etc.. I actually think only getObject > should return null.. Or none of them maybe.. > > However, your idea for getProperty(String ke, Object defaultValue) would it > return an Object? How would that be different then getObject()? > > Also, if you want to donate your JDBCConfiguration, it would be much > appreciated... > > Eric Pugh >
