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 Mon, 10 Sep 2012 16:01:58 GMT
On 9/9/12 11:11 PM, Sébastien Brisard wrote:
> 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.

I agree there, as long as APIs that specify null-handling and
disallow nulls can throw MathIAE in its place.
> 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.

The most important advantage of checking preconditions and throwing
client-meaningful exceptions at the point of activation is that it
makes it easier to diagnose run time failures - not just for
developers debugging code with access to the full sources; but for
operations and other developers who may not have access to the
sources.  Seeing an IllegalArgumentException with an informative
message in a stack trace at the point of activation, anyone can look
at the published javadoc and understand exactly what happened, at
least from the standpoint of the library.  Just seeing an
undocumented NPE somewhere down in the call stack leads to a harder
problem, sometimes requiring added instrumentation, examination of
the source code, or other methods to diagnose what is going on.

Phil
> 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.
>
> 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.
>
> Phil was talking about loss of robustness. I don't think that CM as a
> whole is robust with respect to null pointers. 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