commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <>
Subject Re: [Math] About "NullArgumentException"
Date Thu, 13 Sep 2012 22:43:30 GMT
On 9/12/12 4:10 PM, Gilles Sadowski wrote:
> Hello.
> [For those who don't wish to read the whole post, please at least go towards
> the end and indicate your preferred option. Thanks.]
>>> [...]
>>>> All are bad arguments violating API contract,
>>>> all detected at method activation time -> IAE.
>>> Agreed... if the policy is to _always_ check for "null"!
>> I am only referring to APIs that explicitly disallow nulls.
> I don't know what you mean by "only": Almost _all_ the API disallows "null".
> What has "implicitly" or "explicitly" have to do with that (other than
> "implicitly" being equivalent to lacking documentation vs "explicitly" being
> defined by a default policy)?
>>  I have
>> agreed that we will not always do this.
> Again, this is one point where we differ as regards to "rules". I contend
> that there must be an "ideal", defined by the policy. Is the ideal to always
> check or to never check. I think the latter, for the reasons explained.
> The policy cannot be "sometimes yes, sometimes no" unless you can define in
> which case to do it and in which not, which is clearly more complicated.
>> The specific case being
>> discussed here is an API that is going to include as an explicit
>> precondition that nulls are not allowed.  When this is done, what
>> should be thrown is an IllegalArgumentException, which for us means
>> some subclass of MathIllegalArgumentException.
> If we were to decide that the ideal is to always check (an obviously
> different policy), I wouldn't have a problem with this.
> What I point at is the potential burden on the _user_ if he wants to
> intercept exceptions: for the _same_ problem (null reference), they'd have
> to catch two types of exceptions: NPE vs NAE (as sublcass of MIAE).
> If nobody cares about that, I won't either. I just wanted to stress that we
> might have some user complaints about this inconsistency which will appear
> at the contact point between user code and CM code: Because it is simply not
> nice to report the same problem in different ways.
> And because it is inconsistent, some future contributor will raise this
> issue again: "Why do we check here and throw MIAE and why don't we check
> there and let the JVM throw NPE?". [This is exactly what you reported about
> checking the components of a FieldVector but not the vector itself.]
>>> If we sometimes check and sometimes not, the thrown exception will be from a
>>> different hierarchy (NPE vs MathRuntimeException). This is annoying (and
>>> inconsistent).
>> That is an unfortunate consequence of not always checking.  I
>> understand that it may be impractical to always check.  Therefore,
>> some NPEs are going to be propagated to clients with no warning. 
> What do you mean by "impractical"?
> In practice, we will not check everything tomorrow, but we can decide to
> work toward that goal.
> But we have to answer the question not of whether it is practical, but
> whether it is desirable.
> This is a quite simple issue; and I don't believe in considerations like
> it might be good in some part of CM APIs but not in others. [In _principle_
> I mean; I don't say that in _practice_ some parts of CM won't be up-to-date
> sooner than other parts.]
>>> The oft-repeated issue is "Is it useful to check for null or not?"; at least
>>> 4 people answered "No", because this bug will never go unnoticed: sooner or
>>> later, using "null" _will_ raise exception. The sole and unique difference
>>> with CM checking and JVM checking is that the stack trace will (sometimes)
>>> be a little shorter in the former case.
>> And inferior first failure data capture and potential additional
>> computation, side effects or other unknown consequences (unless we
>> always think through the consequences of nulls propagated through
>> our code and make sure there are never any untoward side effects. 
>> All of these are reasons for the "when you must fail, fail fast and
>> loudly" maxim.  I understand that it may not be possible to adhere
>> to this fully in [math].
> You repeat the mantra "fail fast, etc." but do not give a counter-argument
> to mine about the NPE being as fast as it needs to be for this kind of
> failure.
> I'd like to see one example of "untoward side effects".
>>>  Most people have come to agree that
>>> it's not worth adopting a policy of thorough checking. [Rationale 1: users
>>> who encounter a NPE will need to inspect the stack trace and go to _their_
>>> code in order to see where _they_ put a "null". Rationale 2: there is
>>> probably a performance penalty to all these checks (especially in a well
>>> tested code where "null" reference bugs have been ironed out).]
>> The latter is a good point and why I am OK not checking everywhere.
> The argument is equally valid _everywhere_ (unless you can provide the
> above example), in both directions: either it is good to check, then it is
> always good, or in some cases it is not necessary, then it is never
> necessary.
>>>> Consider, e.g. an empty array or array indices that will lead to
>>>> index out of bounds errors in APIs that take arrays.  What is so
>>>> different about null?
>>> There is no difference... if there is a policy that decrees so.
>>> It is _not_ the policy of the JDK: "NullPointerException" is not the same
>>> error as "IndexOutOfBoundsException"; their common parent class is the
>>> (unspecific) "RuntimeException" and their sibling is
>>> "IllegalArgumentException".
>>> There is no "is-a" relationship between NPE, IOBE and IAE.
>> Here you are missing the point above - we are talking about specific
>> APIs that explicitly disallow nulls.
> I do not think I miss any point. Rather it is you who fail to point to
> to cases where such an "explicit" mention is needed.
> As Luc pointed out, only stating where null _is_ allowed is actually
> useful. And as I pointed out about your example of using null in
> constructors, those occurrences should be very rare, for the very reason
> that such code is much more obscure (and documentation should thus be
> extremely precise to avoid confusion).
>>  Just as we trap or check
>> arguments to avoid array index errors, we can in some cases do the
>> same for nulls.  In these cases, it is appropriate to throw IAE for
>> the nulls just as we do for the (avoided) AIOBE.
>>> Recalling that some 1.5 years ago you were against the adoption of the
>>> single exception hierarchy, rooted at "MathRuntimeException", in order to
>>> stay faithful to the JDK exception types, I wonder why you are on the
>>> opposite stand as NPE is concerned.
>> I am not.  I have agreed that we can go back to a common root.  But
>> that then forces us to create surrogates for the top level
>> exceptions such as IAE that the "math" versions used to subclass.  I
>> am OK with that.  But we should maintain the semantics correctly,
>> throwing a MIAE when API contracts are violated.
> You don't answer to my argument. I did not say that you were against a
> common root _now_, but that you preferred to retain the standard exceptions
> _before_. For the one exception where I think that it is better to indeed
> keep the standard NPE exception, you hold the opposite view (common root).
> In summary, you agree that it might not be good (or "practical", whatever
> that means) to always check for null. Then, where do you draw the line in
> order to take "Rationale 2" into account? IOW, you say that "Rationale 2"
> is a good point but, imagine that someone spends a few days adding all the
> null checks, then this good point will not exist anymore. That's a
> contradiction.
> Again, having NAE inherit from NPE removes all contradictions in one fell
> swoop. I can't fathom why you insist on the naming issue, why this
> precondition should be reported by "MathIllegalArgumentException". I repeat
> that in the JDK "ArrayIndexOutOfBoundsException" is _not_ a subclass of
> "IllegalArgumentException". That, in CM, "OutOfRangeException" _is_ a
> subclass of "MathIllegalArgumentException" is a different choice that _we_
> made. We can make a different choice for "NullArgumentException" if we see
> that there is more benefit: it allows a flexible, yet consistent policy, of
> checking or not checking while throwing the same exception for the same
> problem.
> 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.

> 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:
> For additional commands, e-mail:

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

View raw message