You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Gianugo Rabellino <gi...@apache.org> on 2004/03/31 12:52:33 UTC
[Kernel2.2] Remove nulls?
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?
Just my 0.2c, but since we're talking foundations I think it could be a
right time to be strict about code principles...
Ciao,
--
Gianugo Rabellino
Pro-netics s.r.l. - http://www.pro-netics.com
Orixo, the XML business alliance - http://www.orixo.com
(Blogging at: http://www.rabellino.it/blog/)
Re: [Kernel2.2] Remove nulls?
Posted by Pier Fumagalli <pi...@betaversion.org>.
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
Re: [Kernel2.2] Remove nulls?
Posted by Gianugo Rabellino <gi...@apache.org>.
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:
public Object get(Object name) {
if (name == null) return(null);
o.a.c.kernel.archival.HashLibrary.java:
public Descriptor get(Identifier identifier) {
if (this.contains(identifier)) {
Iterator iterator = this.iterator();
while (iterator.hasNext()) {
Descriptor descriptor = (Descriptor) iterator.next();
if (descriptor.equals(identifier)) return(descriptor);
}
}
return(null);
}
o.a.c.kernel.resolution.CompoundResolver.java:
public Resource resolve(String name) {
Resolver r[] = (Resolver[])list.toArray(new Resolver[list.size()]);
for (int x = 0; x < r.length; x ++) {
Resource s = r[x].resolve(name);
if (s != null) return(s);
}
return(null);
}
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.
Ciao,
--
Gianugo Rabellino
Pro-netics s.r.l. - http://www.pro-netics.com
Orixo, the XML business alliance - http://www.orixo.com
(Blogging at: http://www.rabellino.it/blog/)
Re: [Kernel2.2] Remove nulls?
Posted by Pier Fumagalli <pi...@betaversion.org>.
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?
> Just my 0.2c, but since we're talking foundations I think it could be
> a right time to be strict about code principles...
Absolutely!!!
Pier
Re: [Kernel2.2] Remove nulls?
Posted by Ugo Cei <u....@cbim.it>.
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?
I think it all depends on whether not finding something is to be
considered a normal occurrence or an abnormal condition.
Should java.util.Map.get throw an exception if there's no entry with the
given key? Since the key might be derived from user input, not finding
the entry should be considered a possible occurrence and not an error,
so returning null is a better option than throwing an exception, IMHO.
Conversely, if the thing you're looking for _must_ be there, not finding
it is clearly a programming error, so you should throw an _unchecked_
exception.
I would avoid checked exceptions in any case.
Since I haven't looked at the Kernel code, I cannot tell if we're
talking about the former or the latter case.
As for the NullObject pattern, I have never used it, so I won't comment
on it.
> Just my 0.2c, but since we're talking foundations I think it could be a
> right time to be strict about code principles...
Just my 0.02€ (so this is worth 10 times your opinion, or about 12 if
you're talking dollars ;-) ).
Ugo