commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Barker" <>
Subject Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API
Date Wed, 02 Mar 2011 03:16:40 GMT

-----Original Message----- 
From: Gilles Sadowski
Sent: Tuesday, March 01, 2011 3:12 PM
Subject: Re: [Math - 403] Never propagate a "NullPointerException" resulting 
from bad usage of the API

>> It's a debate that goes on. Josh Bloch in his Effective Java book says 
>> NPE
>> is perfectly acceptable for bad arguments. So it really depends on your
>> perspective what an NPE represents. I prefer Josh's opinion but only 
>> because
>> every single argument probably creates lots of branch-checking that kills
>> cpu pipelining.
>> > As far as this issue is concerned (for what i have understood) i 
>> > believe
>> > that one way to separate NULL(s) that occur from the A.P.I. from 
>> > NULL(s)
>> > coming from wrong usage of A.P.I. by a user is the assert technique... 
>> > I
>> > didn't know a lot about it but from what i have read it should be
>> > implemented only in the private methods of the A.P.I. Check this link 
>> > out:
>> > "
>> >".
>> > Another choice is to create a new class that would check all the 
>> > arguments
>> > of every function we are interested in (for example: public
>> > checkArguments(Object... args)) [If i have understood correctly the 
>> > purpose
>> > of this issue...]. Any suggestions would be more than welcomed!
>NPE is the symptom of a bug.
>Using "NullArgumentException" instead of the standard NPE so that the CM
>exception hierarchy is singly rooted (the root being "MathRuntimeEception"
>in the development version). An advantage is that it is easy to determine
>whether an exception is generated by CM. A drawback is that it is
>non-standard but this is mitigated by the fact that all other exceptions 
>also non-standard (e.g. "MathIllegalArgumentException" instead of IAE).
>One has to take into account that we settled on this choice because it 
>it somewhat easier to implement other requirements (namely the localization
>of the error messages). It's a compromise (without the localization
>requirement, I would have favoured the standard exceptions). And, apart 
>avoiding code duplication, this choice has some "features" (which might be
>construed as advantages or drawbacks, depending on the viewpoint)...
>I'm not sure of what you mean by "branch-checking", but I don't think that
>checking for null makes the problem bigger than it would be otherwise, 
>CM already checks for many things.
>In the end, I'm really not sure what is the best approach for this
>particular case. Personally, I'd be happy that the CM code never checks for
>null and let the JVM throw NPE. This would hugely simplify the CM code,
>albeit at the cost of detecting bad usage a little later. IMHO, it is not a
>big deal because the bug is that an object is missing somewhere up the call
>stack, and it should be corrected there...

I'm in favor of just letting the JVM throw NPE.  Since there is no message 
in this case, there is nothing to localize.  Using a class to check 
arguments is too much work, since the (localized) message "Something was 
null" is less than helpful.  And assert will be turned off in any reasonably 
configured production server so makes the code less readable and adds very 
little value.  If the null happens because of code in CM (as opposed to user 
error), then we'll get a Jira issue, fix it, and add a unit test.  If it is 
user error, then the stack trace of the NPE will tell the developer what was 
wrong in at least 95% of the cases.

>Of course, this would mean that MATH-403 should be dropped, the
>"NullArgumentException" class removed, and the policy changed to: "Never
>check for null references".

To unsubscribe, e-mail:
For additional commands, e-mail: 

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message