commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Luc Maisonobe <Luc.Maison...@free.fr>
Subject Re: [Math] About "NullArgumentException"
Date Mon, 10 Sep 2012 10:27:24 GMT
Le 10/09/2012 08:11, Sébastien Brisard a écrit :
> Hi,
> 
> 2012/9/10 Gilles Sadowski <gilles@harfang.homelinux.org>:
>> On Sun, Sep 09, 2012 at 09:16:51AM -0700, Phil Steitz wrote:
>>> On 9/9/12 4:34 AM, Gilles Sadowski wrote:
>>>> Hi.
>>>>
>>>> Further discussion on the JIRA page
>>>>   https://issues.apache.org/jira/browse/MATH-856
>>>> cannot reach a consensus on solving the issue that raised this thread.
>>>>
>>>> The proposal was that CM never checks for "null" and lets the JVM do it (and
>>>> thus throw the standard NPE).
>>>>
>>>> Phil wants to retain some null checks but opposes to throwing a NPE without
>>>> a "detailed message".
>>>> The localization mechanism being implemented in "ExceptionContext", we
>>>> cannot throw a standard NPE (since the error message won't be localized).
>>>>
>>>> For a consistent behaviour (as seen from the caller), we would have to
>>>> implement a subclass of the standard NPE: callers could do
>>>>
>>>>  try {
>>>>    // Call CM
>>>>  } catch (NullPointerException e) {
>>>>    // Handle NPE (raised by the JVM _or_ by CM).
>>>>  }
>>>>
>>>> However, this breaks the consensus we arrived at (for v4.0) about CM
>>>> throwing only subclasses of "MathRuntimeExceprion" (singly rooted hierarchy).
>>>>
>>>> Phil proposes to throw MathIAE (IMO, it must be the specific
>>>> "NullArgumentException"), but this breaks the above use-case: Users have
to
>>>> do
>>>>
>>>>  try {
>>>>    // Call CM
>>>>  } catch (NullPointerException e) {
>>>>    // Handle NPE (raised by the JVM).
>>>>  } catch (NullArgumentException e)
>>>>    // Handle NPE (raised by CM).
>>>>  }
>>>>
>>>> showing blatantly that CM is not consistent: sometimes it lets a JVM
>>>> exception propagate, and sometimes it catches the problem eralier and throws
>>>> an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0,
>>>> "MathRuntimeException").
>>>> This is the current state of affairs, and I think that it is not
>>>> satisfactory. [As proven by this issue having recurred two or three times
>>>> already.]
>>>>
>>>> In light of this, I propose that either
>>>> * Phil changes his mind (no check for null performed in CM code), or
>>>> * we make an exception to the singly-rooted hierarchy just for
>>>>   "NullArgumentException" (which, in 4.0, would become a subclass of the
>>>>   standard NPE).
>>>
>>> Why not just leave things alone [...]
>>
>> For the reason I gave above: the inconsistent/non-existent policy will make
>> the issue recur sooner or later, as it happened now with Sébastien, as it
>> happened with me when I first signalled the burden of checked excpetions and
>> later when we agreed about MathRuntimeException, then changed again, to come
>> back again now, to where we were almost two years ago (IIRC)...
>>
>>> - i.e., let some APIs document null
>>> handling and throw IAE at the point of method invocation when
>>> supplying a null violates the documented API contract?
>>
>> The answer to that question is in the previous post.
>>
>>> We can leave the (needless, IMO) NullArgumentException as a subclass
>>> of MathIAE in place, or drop it and throw MathIAE directly in these
>>> cases.
>>
>> "NullArgumentException" is about as needless as any subclass of "Exception"
>> or "RuntimeException". Either we use inheritance for what it's primarily
>> meant or we choose another, non-OO, language.
>>
>> This is going in circles; some people will drop from the discussion (or
>> already did) and some time from now, someone will "rediscover" this, among
>> other little defects of CM. These are worth correcting, but not worth
>> discussing endlessly.
>>
>> Let's just have all people here provide their preference and be done with
>> it.
>>
> Since there is no way to enforce a strict policy on checking for null
> in CM, I think that NAE is useless, and should be droped.
> If we assume that every user of CM can use a debugger, then probably
> (and contrary to what I advocated earlier), checking early for null is
> also superfluous.

There are some cases pointed to be Phil which can be useful. We can let
them if they are already there.

> I tend to document arguments which *can* be null (I think someone else
> also mentioned this practice), so that it's fairly safe (as someone
> already wrote) to assume that all other arguments *must* be non-null.

+1.

> 
> To sum up, I would favor complete removal of NAE. As for existing
> checks, I would either remove them, or throw an argumentless standard
> NPE.

As null is so ubiquitous, I would tend to choose Gilles second option
(i.e. we preserve NullArgumentException, outside of the single root
hierarchy I promote for other exceptions). This would preserve
robustness Phil asks for and I would not complain against this as in
this case I would not mind if it is not advertised in javadocs and
throws declarations (because even if we don't throw it ourselves, a
regular NPE from JVM can always occur).

> 
> Phil was talking about loss of robustness. I don't think that CM as a
> whole is robust with respect to null pointers.

It is not globally robust, but in some places it is because some effort
was already put on this.

Luc

> In some places, the
> code fails in the standard way (NPE), while in others, it fails in a
> fully documented way. Since this is inconsistent, I don't think we
> should be afraid of losing robustness in the case of complete removal
> of existing checks for null. Again, I'm happy to keep them, but I'd
> rather throw a standard NPE in this case.
> 
> Sébastien
> 
>>
>> Gilles
>>
>> P.S. Is there an occurrence in CM, where a method can be passed a null
>>      argument?
>>
>>
>>>
>>> Phil
>>>
>>>>
>>>> The second option cares for all the various positions _except_ the
>>>> singly-rooted hierarchy.
>>>>
>>>>
>>>> Regards,
>>>> 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
>>
> 
> 
> ---------------------------------------------------------------------
> 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