On 31 Mar 2004, at 13:34, Gianugo Rabellino wrote:
Pier Fumagalli wrote:
On 31 Mar 2004, at 11:52, Gianugo Rabellino wrote:I've seen in the new API that in a number of places there are contracts that return null if something isn't found: since these are all perfect candidates for nasty NPEs, how about switching to NullObject pattern or throw Exceptions instead?As far as I can see, "null"s are returned only when implementing the collections interfaces (Library, Parameters, Configuration) or when URLs are resolved (Resolver).
And even there (ex. Configuration and Parameters) when calling a method specifed in the interface, and not inherited from the Set/Map/Collection, it will throw specific exceptions (ex. ConfigurationException).
Where did you notice it?
Well... there are 10 occurrences, but this are the most prominent:
o.a.c.kernel.configuration.Parameters.java:
o.a.c.kernel.configuration.Parameters implements java.util.Map;
there are a ton of "getParameter..." methods in there throwing the right exceptions. We could add a generic "getParameter" that behaves like you want and that breakes the "get" contract with Map
o.a.c.kernel.archival.HashLibrary.java:
o.a.c.kernel.archival.Library implements java.util.Set:
as the set doesn't have a "get" method, we could throw an exception, but if it does I'd rather call the method "getDescriptor()" (to show that it is _really_ outside of the concerns of Java Collections)
o.a.c.kernel.resolution.CompoundResolver.java:
This follows the "null" returned by the usual source resolution mechanism used by the platform. See:
http://java.sun.com/j2ee/1.4/docs/api/javax/servlet/ ServletContext.html#getResource(java.lang.String)
http://java.sun.com/j2se/1.4.2/docs/api/java/lang/ ClassLoader.html#getResource(java.lang.String)
Only Avalon seems to be throwing an IOException:
http://avalon.apache.org/excalibur/api/org/apache/excalibur/source/ SourceResolver.html#resolveURI(java.lang.String)
Which is my very own opinion is wrong because an IOException signifies that there was an error in the I/O (the remote site was down, the process didn't have enough access to read from the disk). The correct Exception to throw would be
http://java.sun.com/j2se/1.4.2/docs/api/java/util/ MissingResourceException.html
I _really_ don't like reinventing wheels! :-P (whops, I just did)
I haven't been through the code to see what exactly these things do, but from a pure OOP POV (whatever that means :-)) there is some "code that smells" (http://c2.com/cgi/wiki?CodeSmell). It might make sense, but in most cases it doesn't.
My armpits can smell pretty badly too at times...
Pier
smime.p7s
Description: S/MIME cryptographic signature
