commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API
Date Wed, 02 Mar 2011 12:35:16 GMT
On 2 March 2011 08:20, Paul Benedict <pbenedict@apache.org> wrote:
> BTW, you can find precedence in the JVM for many methods that throw NPE on
> null arguments. I am not saying this is the "right way", since such things
> are subjective and are a matter of design, but many people have concluded
> it's better.

If the NPE would not be detected until the method has done some other
work, then I can seem why one might want to detect it earlier.

And the line number may be insufficient to identify the source of the
NPE - there could be several de-references in a single line.

> On Tue, Mar 1, 2011 at 9:16 PM, Bill Barker <billwbarker@verizon.net> wrote:
>
>>
>>
>> -----Original Message----- From: Gilles Sadowski
>> Sent: Tuesday, March 01, 2011 3:12 PM
>> To: dev@commons.apache.org
>> Subject: Re: [Math - 403] Never propagate a "NullPointerException"
>> resulting from bad usage of the API
>>
>>
>>  Hi.
>>>
>>>  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:
>>>> > "
>>>> > http://download.oracle.com/javase/1.4.2/docs/guide/lang/assert.html".
>>>> > 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
>>> are
>>> also non-standard (e.g. "MathIllegalArgumentException" instead of IAE).
>>> One has to take into account that we settled on this choice because it
>>> makes
>>> 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
>>> from
>>> 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,
>>> since
>>> 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".
>>>
>>>
>>> Best,
>>> Gilles
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>

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


Mime
View raw message