Re: [vqwiki-dev] Current Status, Task Requests and Project Direction
Status: Abandoned
Brought to you by:
mteodori
From: Ryan H. <rya...@gm...> - 2006-06-08 07:34:18
|
Hi Martijn, I've just gotten around to trying to build the project to see where the current issues are, and took some time to review the changes to the Environment.java class. I very much like how you've simplified the class, but I do have two concerns about removing the PROPERTY_NAME constants and moving the property value defaults into a property file. First, without having constants for the PROPERTY_NAME values the implementation will then involve hard-coding the propery names, eg: env.getValue("some-property"). That creates an instance where something that might have been caught as a compile error becomes a (harder to debug) runtime error. For example, env.getValue(PROPERTY_DB_SERVER) isn't susceptible to typos, but env.getValue("db-server") would be. Moving default property values into a property file is something that I'm less opposed to, although as a programmer I like being able to easily see what the variables are being initialized to. Also, since these values are used only to initialize property values, hard-coding isn't really a problem (it's the same scenario as initializing any other variable - the value isn't going to be used elsewhere). Re-adding each of these items wouldn't complicate the class too much, as it would simply mean including several constants and adding a static initializer for the "defaults" properties object. Assuming that these changes were motivated by a desire to make the properties system more generic, a plugins.properties file could always be added if needed and included in the readProperties() method. That would have the additional advantage of keeping core & third-party properties separate. As always, let me know if I've missed something or if you have a different opinion. Again, I really like how you've simplified the class - it's vastly easier to read than the previous version. Cheers, Ryan Martijn van der Kleijn wrote: > - A default.properties file was introduced with the intention of removing > hard-coded default values. > - Major changes in Environment class to match introduction of > default.properties.... THIS IS CURRENTLY VERY BROKEN |