commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 34103] - [configuration] Inconsistent behavior
Date Tue, 22 Mar 2005 06:45:43 GMT
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=34103>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=34103





------- Additional Comments From ittayd@qlusters.com  2005-03-22 07:45 -------
i totaly agree with KISS, which is my point:
1. a getString() which takes the first element of a list is not KISS - a kiss is
that getString returns the property converted to a string. to put it in another
way, if my property is "a,b,c" i'll get "a" in return, but if it is "a-b-c",
i'll get "a-b-c". why? because i used '-' instead of ','? a KISS is that i'll
get 'a,b,c' or 'a-b-c' respectively - i get the property as-is, converted to
string. that way your model is very clear: 'getXXX() returns the property
converted to XXX', you're current model is: 'getXXX() returns the property
converted to XXX, unless the property is a List, in which case it returns the
first element'. moreover, in your current documentation this isn't even
described properly, for example, it is not mentioned for getBoolean(). 


2. about throwing an exception: the thing is you do throw this exception in many
cases, sometimes depending on a flag, sometimes not.

to summarize, my KISS model for properties is:
1. there are two types of properties: mandatory and optional. mandatory
properties mean that the client has to define them, there's no default. for
example, a URL of some server. for such properties, the code uses getXXX(key)
methods, which throw an exception if the property doesn't exist (which the code
can then catch, print an error and exit). optional properties are ones where the
code allows the client to override some default value. for this the code uses
getXXX(key, default), which doens't throw an exception if the property doesn't
exist. no flags. this is, i believe, very natural. if i have a default, i use
the optional form, if not, i use the mandatory form. the framework doesn't try
to guess defaults for me.

2. the model for the return value is always 'getXXX() returns the property
converted to XXX' an exception is thrown if the conversion fails. this exception
is a runtime exception, because it means some logical fault in the code.
conversion is done on the whole property value, not part of it. that way, a
client always knows what he's getting. if he wants the first value of a list
property he can get it like getList(key).get(0). very simple.

3. conversion is done in such a way so that custom converters can be added. that
way, if i want to get a property converted to MyClass, i call getProperty(key,
MyClass.class). simple. there's no alternative in the framework today, because i
have to work with what the interface in Configuration gives me, which will no
doubtly blow up as users will reqruie more getXXX methods.


1 & 2 give a very simple model to use. look also at bug # 34098, for a more
detailed in how the current implementation is far from KISS and confuses the
user by not being consistent in the values it returns (this is not purely
academic, but as a result of bugs i faced because, for example, i expected
getProperties(key) to return null if key doesn't exist, but instead got an empty
properties list, maybe convenient in some cases, but not KISS - no standard
returned value) 

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message