commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: [Math] About "NullArgumentException"
Date Fri, 14 Sep 2012 18:29:41 GMT
OK, I give up.  Lets do option 2.  Just warn users in the User Guide
somewhere that our APIs are in general not null-safe and unless the
javadoc explicitly allows nulls, they can expect NPE when passing nulls.

Phil

On 9/14/12 8:46 AM, Gilles Sadowski wrote:
> On Fri, Sep 14, 2012 at 07:44:08AM -0700, Phil Steitz wrote:
>> On 9/14/12 4:28 AM, Gilles Sadowski wrote:
>>> On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote:
>>>> Le 14/09/2012 08:46, Sébastien Brisard a écrit :
>>>>> Hi Phil
>>>>>
>>>>>>> Back to square one, with 3 fully consistent alternatives:
>>>>>>>  1. CM to always check for null? Then "NullArgumentException"
inheriting from
>>>>>>>     "MathIllegalArgumentException" is fine because we promise
to the user that
>>>>>>>     no NPE will ever propagate through the CM layer. [Breaking
that promise
>>>>>>>     is a CM bug.]
>>>>>>>  2. CM to never check for null? Then we delete class "NullArgumentException".
>>>>>>>     Users are warned by the general policy: "Do not pass null
unless it is
>>>>>>>     explicitly documented to be allowed." A bug will lead to
the JVM raising
>>>>>>>     a NPE.
>>>>>>>  3. CM to sometimes check for null? Then "NullArgumentException"
should
>>>>>>>     inherit from "NullPointerException" because the user will
sometimes see
>>>>>>>     "NullArgumentException" (when CM checks) and sometimes NPE
(when CM does
>>>>>>>     not check) and both should thus belong to the same hierarchy
(from the
>>>>>>>     user's point-of-view, there is no reason to handle them in
different
>>>>>>>     ways since it's the exact same bug).
>>>>>>>     For the user, the consequence will be similar to alternative
2, with
>>>>>>>     sometimes more information about the failure and sometimes
(marginally)
>>>>>>>     earlier detection.
>>>>>> As stated above, my preference is
>>>>>>
>>>>>> 4.  CM APIs *may* disallow nulls explicitly.  When that is done,
the
>>>>>> implementations *should* check parameters (as they *should* check
>>>>>> all other stated preconditions) and when preconditions are violated,
>>>>>> a MathIllegalArgumentException is thrown.  I don't care whether or
>>>>>> not we keep NAE.  If we drop it, we should make sure whatever
>>>>>> exception messages we used to use to indicate illegal null arguments
>>>>>> are still there.
>>>>>>
>>>>>> Phil
>>>>>>
>>>>> I like this option better than 3. So, I'll change my "vote" to option
>>>>> #2, and possibly option #4 as we will never agree on option #2.
>>> Why is it better to throw MathIllegalArgumentException rather than NPE?
>> Because, as I have repeated numerous times, that is the appropriate
>> exception to throw when API preconditions are violated.  That is why
>> IAE exists.  The class javadoc comment for IAE is nice and succinct
>> (we could learn from it ;)
> IllegalArgumentException:
> ---
> Thrown to indicate that a method has been passed an illegal or inappropriate
> argument. 
> ---
>
> Did you read the class Javadoc for NPE?
> Here is the last sentence pasted again:
> ---
> Applications should throw instances of this class to indicate other illegal
> uses of the null object. 
> ---
>
>>   You could ask the same question about
>> ArrayIndexOutOfBounds.  Why not throw that instead of IAE when bad
>> indices are provided?
> Does CM check _array_ index? No.
> Surprised? An "ArrayRealVector" is _not_ an array, it is the representation
> of the vector concept. That it is a backed by a Java array is an implemtation
> detail that should not surface to the API. Hence the choice to not use the
> low-level ArrayIndexOutOfBoundsException can be given a rationale.
> CM checks that you access valid entries of the high-level "mathematical"
> object. And we do that for _all_ such accesses and never (or it is a CM bug)
> let propagate an exception that reveals an underlying Java array.
> [This was one of my proposals too (alternative 1). However it imposes a
> unnecessary burden of duplicating (CM and JVM) every null check just for the
> sake of hiding NPE.]
>
> As indicated in a previous post, "null" is not a "RealVector", it is not an
> "Object", it is not a reference, it is just the value of an unitialized
> pointer.
>
>>   The difference is that instead of letting
>> the precondition failure go undetected and the runtime exception
>> propagate, APIs with explicit preconditions and parameter checking
>> in place raise the exception at method activation time.
> Could you please stop mentioning this, since my proposal (alternative 3)
> allows for early detection?
>
> The difference is that you want to use MathIAE, where everyone else uses NPE.
>
>>  In that
>> context, what is appropriate is IAE.
> You are stuck with a "name", as if everything that is "illegal" must be
> signalled with an exception that contains the string "Illegal" in its name.
>
> If that is so, I propose to rename "NullArgumentException" to
> "IllegalNullArgumentException".
>
>
> Gilles
>
>> Phil
>>>> My preferences are 4 or 3 with same preference level and 2 as a fallback
>>>> choice. For each option, I find the arguments are good, it's only a
>>>> matter or preference.
>>> Did everyone consider that Phil's alternative implies a behaviour that
>>> departs from the standard practice, a.o. 
>>>  * JDK (cf. quote in a previous post),
>>>  * other libraries, e.g. Guava (cf. quote in a previous post), and
>>>  * Java experts, e.g. Joshua Bloch[1]
>>> of throwing NPE when they encounter "null"?
>>> Doesn't anyone care about having a _reason_ for CM to behave differently?
>>>
>>> Don't you care that users will see different exceptions when the same
>>> problem is detected?
>>>
>>> Do you really like a policy that mandates different behaviours when "null"
>>> is disallowed implicitly vs explicitly (throwing NPE vs throwing
>>> MathRuntimeException)?  [Pardon me, but IMHO this is nonsense.]
>>>
>>> After piling up arguments (_none_ of which have been addressed), I'm sorry
>>> to read that it would only be a "matter of preference"! [I should start a
>>> career as a writer if I'm able to express "preference" in so many words...]
>>> Actually, it's a matter of consistency ("same problem, same exception").
>>> If nobody cares about improving CM's consistency, then indeed most of the
>>> discussions in which I take part are pointless.
>>>
>>> What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows
>>> CM to fail as early as possible (which I thought was Phil's main point, as
>>> it was his only one, apart from "preference"), while being consistent ("same
>>> problem, same exception"), internally, externally, implicitly and
>>> explicitly. Alternative 4 is inconsistent ("same problem, different
>>> exceptions"); it's a truth, not a matter of taste.]
>>>
>>> Rather then being willingly stuck in a deadlock because only the weakest
>>> argument ("preference") is taken into account, wouldn't it be more
>>> reasonable to count all the arguments?
>>>
>>>
>>> Gilles
>>>
>>> [1] "Effective Java" (second edition). Excerpt from "Item 60":
>>>     If a caller passes null in some parameter for which null values are
>>>     prohibited, convention dictates that NullPointerException be thrown
>>>     rather than IllegalArgumentException.
>>>
>>>> Luc
>>>>
>>>>> Best regards,
>>>>> Sébastien
>>>>>>> Your alternative (sometimes check, sometimes not) is not fully
consistent:
>>>>>>>  * for the user: "In case of null reference, will I get a MathRuntimeException
>>>>>>>    or a NPE?"
>>>>>>>  * for the CM developer: "In which cases do I need to check for
null?"
>>>>>>>
>>>>>>> Of course, I would reconsider if you could provide an actual
example that
>>>>>>> would fail with all three alternatives which I suggested. If
not, then
>>>>>>> it seems obvious that your alternative presents two defects that
don't exist
>>>>>>> in any of those three.
>>>>>>>
>>>>>>>
>>>>>>> Gilles
>>>>>>>
>>>>>>>>>> [...] what is different about null arguments at the
point of
>>>>>>>>>> method activation in an API that explicitly disallows
them.
>>>>>>>>> The difference is that there is no need to tell the user
what the problem
>>>>>>>>> is because the raised exception says it all: "You tried
to use a null
>>>>>>>>> reference." [As said above, the only issue is _when_
the exception is
>>>>>>>>> triggered.]
>>>>>>>>>
>>>>>>>>> The policy mandates what is globally valid, e.g.:
>>>>>>>>>   "If not specified otherwise, "null" is not allowed
as an argument."
>>>>>>>>> Then, a user cannot complain about a NPE being propagated
when he passed an
>>>>>>>>> invalid (null) argument.
>>>>>>>>>
>>>>>>>>> Finally, the case for "null" is also slightly peculiar
(given the above)
>>>>>>>>> that it should not simply be bundled with the rationale
"Fail early": Indeed
>>>>>>>>> "null" references always fail early (i.e. as soon as
they are used).
>>>>>>>>> Deferring the check until it is done by the JVM will
never entails the code
>>>>>>>>> to produce a wrong result _because_ of that null reference
(it will just
>>>>>>>>> fail in the predictable way: NPE).[1]
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Gilles
>>>>>>>>>
>>>>>>>>> [1] Unlike in C, where an unitialized pointer would not
necessarily crash
>>>>>>>>>     the program, but _could_ make it behave erratically
(including produce
>>>>>>>>>     wrong results in a stealth way).
>>>>>>>>>
> ---------------------------------------------------------------------
> 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