RE: [configuration] AbstractConfiguration

[Eric Pugh <ep...@upstate.com>]

Re: [configuration] AbstractConfigurationKonstantin,

I have applied your patchs..  Can you do a fresh CO and just verify I
applied everything properly.  The unit tests all run cleanly for me.  Can't
wait to see your JDBC configuration!

Eric
  -----Original Message-----
  From: Konstantin Shaposhnikov [mailto:k8n@tut.by]
  Sent: Tuesday, September 30, 2003 1:08 AM
  To: 'Jakarta Commons Developers List'
  Subject: Re: [configuration] AbstractConfiguration


  Eric,

  I perform code formatting of my changes to sources. See
  attached patch file. I also use Eclipse for the development.
  Unfortunately I am behind firewall, so I can't access cvs
  from eclipse (because for some reason it refuses to work with
  socks5 proxy server), so I create patch file using cvs diff
  command. Then I try to apply this patch to the unmodified
  sources using Eclipse and everything looks fine.

  As for checkstyle I think that It report a lot of useless
  warning. F.e. about final parameters. May be we can modify
  checkstyle configuration to turn them off or just ignore
  them. Now AbstractConfiguration has 175 warnings and
  BaseConfiguration - 21.

  I modify testBoolean to return Boolean instead of String.

  As for additional tests it seems to me that existing tests
  cover addProperty/addPropertyDirect functionality. Actually
  this tests were very helpful for me when I perform
  refactoring.

  Also I think that later I can refactor other Configuration
  classes to extend AbstractConfiguration.



  On 17:28 Mon 29 Sep     , Eric Pugh wrote:
  > [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:k8n@tut.by]
  >   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
  >   >
  >
  >

Re: [configuration] AbstractConfiguration

[Konstantin Shaposhnikov <k8...@tut.by>]

Hi Eric,

Thank you for applying patches. Everything looks good for
me. 

Unfortunately I don't have a lot of free time now because of
my work and studying. So I can't perform any promised
changes and additions in the near future. But I hope that
I'll send you some patches as soon as I have some free time.

As for JDBC Configuration I still have no clean idea how this
class should work. It require a lot of parameters, such as
name of the table with configuration properties, filed names
for keys and values for proper working, database connection
properties. Should it allow storing several configuration
values under one key (as PropertiesConfiguration)? If you have
some ideas about this type of Configuration please describe
them.

Konstantin.