Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: neutral (athena.apache.org: local policy)
Message-ID: <4D6F7512.5070308@free.fr>
Date: Thu, 03 Mar 2011 12:01:38 +0100
From: Luc Maisonobe <Luc.Maisonobe@free.fr>
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US;
 rv:1.9.2.13) Gecko/20101208 Thunderbird/3.1.7
MIME-Version: 1.0
To: Commons Developers List <dev@commons.apache.org>
Subject: Re: [Math - 403] Never propagate a "NullPointerException" resulting
 from bad usage of the API
References: <AANLkTikfv8NTprVs0Xso9Kpm+j_5YT=1KoZ=+e2O68t=@mail.gmail.com>
 <AANLkTi=VaTw2bfvADOWgyxM0RLNU6BF7ki-8F_Gadj5=@mail.gmail.com>
 <20110301231236.GI22170@dusk.harfang.homelinux.org>
 <514CE0C1587E41418D26F4AE7EBAC6EA@BillPC>
 <AANLkTi=rT=ums11F_56A=tQuUnQW9kLoscF58xo95TGL@mail.gmail.com>
 <AANLkTinbDWaTfJQEw0CCug64+aT9mxR3od7dtcG3Xcrj@mail.gmail.com>
 <20110302113717.GN22814@dusk.harfang.homelinux.org> <4D6E46FB.80608@free.fr>
 <20110302230832.GJ22170@dusk.harfang.homelinux.org>
In-Reply-To: <20110302230832.GJ22170@dusk.harfang.homelinux.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8bit

Le 03/03/2011 00:08, Gilles Sadowski a �crit :
> Hello.
> 
>>>> The recognised standard in the JDK is NPE on invalid null input.
>>>>
>>>> I have never overly liked this, but conform to it for JSR-310/ThreeTen.
>>>>
>>>> I use IAE in other projects which are higher up the stack. Whether
>>>> [math] is low or high level may determine the choice you make.
>>>
>>> I've always heard that CM is "low"-level; and this serves as the basis of
>>> many arguments (e.g. the "no-dependency" one).
>>
>> Yes.
>>
>>> However I think that, in CM, the line between low- and high-level is blurred
>>> when some features are concerned (e.g. IMO localization does not belong in a
>>> low-level library).
>>
>> It is a requirement for many libraries. In fact I even saw this
>> requirement being explicitly written in the specifications of an
>> official public Request For Proposal for a project concerning Apache
>> Commons Math.
> 
> IIRC, one of your clients required localization.
> But if the majority of the opinions expressed on this ML were any sign,
> there wouldn't be a localization feature in CM.
> 
> Now, I'm not advocating to remove it (in the previous discussion, in reply
> to people who said it was unnecessary, I stated that it should be treated as
> a requirement).
> I still don't like the whole idea, but the implementation has much improved
> (from the situation where the pattern strings were duplicated in every
> exception raised).
> The new hierarchy was partly an outcome of this improvement; and, since it
> was another requirement to have very detailed error messages, I had proposed
> to have a hierarchy of stateful exception (because I find it more convenient
> to read a more specific exception name in the stack trace rather than a
> plain "IllegalArgumentException" with a long message).

I agree with that. I se good reasons for the new organization.

> 
> Out of curiosity, where can I read that "Request For Proposal"?

Sorry, you cannot read it. It's access is restricted to the company who
were previously selected in a frame contract. It's for a public agency,
were we met a few months ago (but not for the same people).

> 
>>>> Personally, I don't use NullArgEx, as it is never an exception that
>>>> the user should catch. The message of IAE is sufficiently good to
>>>> remove the need for NullArgEx. Thus in my opinion, [math] would be
>>>> better off without NullArgEx (as it adds no real value).
>>>
>>> I guess that not everyone would agree with the "never". An application
>>> requirement might be that only a certain kind of exception is expected.
>>> Thus calls to CM are wrapped and every exception that comes out is turned
>>> into one of the "allowed" exceptions.
>>> I don't discuss whether it's good or bad programming style, I just mention
>>> that it happens.
>>>
>>>> However, with this, and other exception issues, the truth is that for
>>>> [math] its mostly a matter of taste. The value in [math] is in the
>>>> algorithms, not in whether the exceptions are any good or not. As
>>>> such, I would advise worrying less about exceptions, and more about
>>>> maths. If there is an exception decision, just try to follow the
>>>> modern JDK examples where possible (as it reduces discussion here).
>>>
>>> My viewpoint is that an exception is, in essence, a low-level object. It
>>> should just remain a very convenient way to communicate failure from a lower
>>> to upper layers of code. My revamping the CM's exceptions aimed at regaining
>>> the straightforward usage of standard exceptions (i.e. a direct call to
>>> "new" for constructing an exception that conveys the unadorned reason for
>>> the failure).
>>> The compromise we had reached was about keeping the localization feature,
>>> which in turn implied a departure from the standard exceptions in order to
>>> avoid code duplication.
>>
>> Not really. Typically the 2.1 method
>> MathException.createNullPointerException (and the other similar ones)
>> did achieve using both the standard exception and the localization at
>> the same time.

I'm OK with the change.

> 
> No, that implementation is not "straightforward usage". This factory is
> unwieldy (and it is an abuse of the "Factory" design pattern). I'm not
> going to repeat all the arguments against it; just compare the same "throw"
> statements "before" (release 2.2) and "after" (trunk)...
> 
>> So I would say localization and exception hierarchy are
>> completely orthogonal features.
> 
> "Localization" complicates everything, thus also the exceptions (especially
> so in CM, because localization is used there).
> But localization is not even the main problem, it was the attempt to replace
> a language construct (the exception) by a series of strings (or now enums),
> many of them were duplicates (indeed, there are already so many of them that
> when a "new" message was needed, it seemed that it was added without
> thoroughly checking whether an existing one could be reused).
> The new exceptions were also aimed at relieving this problem.

I agree there is duplication and there are too many messages.

> 
>>> I consider this more important than using the
>>> standard exceptions hierarchy because, in CM, most exceptions are not
>>> recoverable anyway and they mainly serve by showing a failure message (i.e.
>>> at that point, whether an object is an instance of this or that class does
>>> not matter anymore).
>>>
>>> The discussion flared up when we started arguing on semantical issues that
>>> (IMHO) exceptions were not designed to handle and cannot enforce. I'm not
>>> going to re-state all the arguments once again; I'll simply quote you:
>>>   "The value in [math] is in the algorithms, not in whether the exceptions
>>>    are any good or not."
>>> I totally agree with the first statement, of course.
>>
>> Fine! I think we all do agree on this.
>>
>>> The second part does
>>> not add (nor subtract) any value to it; it's unrelated. In my view, the
>>> exceptions are good if they allow to easily track down bugs (be they in CM
>>> or in user code). Accordingly they must precisely point to the source of the
>>> problem (in the code) and not try to outguess the user (as to what it
>>> should mean for him).
>>
>> So can we try to conclude this part and go back to algorithms ?
> 
> All I have been asking is to take the simplest approach: Let's
> first implement what is obviously needed (i.e. low-level exceptions like
> "NotFiniteNumberException", "OutOfRangeException", etc.) and then we'll see
> how it goes and if we really need those higher-level concepts.
> It will be easy to wrap a low-level exception into higher-level ones.
> 
>>
>> Rereading the past threads about this, including the advices from
>> non-math people, I would propose the following consensus :
>>
>>  1) use only unchecked exception
>>     (i.e. confirm the switch started in trunk)
> 
> +1
> 
>>  2) keep localization
> 
> -0 (or +1, if we consider it as a requirement).

It's a requirement.

> 
>>  3) don't put all exceptions in a dedicated exception package
>>     (but the general exceptions used everywhere could go there)
> 
> -0 because most exceptions are general and thus I don't see the benefit of
>    scattering the remaining few among several packages.
> 
>>  4) put specific exceptions in the package they are triggered
>>     (for example ODE, linear ...)
> 
> -1 unless we can be convinced that the specific exception has no usage in
>    another package (now and in the foreseeable future).

SingularMatrixException, NonSymmetricMatrixException and the likes are
really tightly bound to linear algebra and could be in the linear
package where they are triggered. They could appear in the signatures of
algorithms in other package that do call linear algebra, but this is not
sufficient to put them in a general package.

> 
>>  5) use a small hierarchy backed by an implementation of the principle
>>     of exception context as per [lang] to provide state information
>>     and allow extension by users calling addValue(label, value),
>>     a typical example could be one ConvergenceException class and
>>     different labels/values for count exceeded or NaN/Infinity appearing
> 
> -1 for removing any of the new exceptions (except "NullArgumentException"):
>    The hierarchy in trunk is shallow and small.
> 
> +0 for implementing the map feature.
>    Do you mean it as replacement of the "general" and "specific" message
>    pattern (i.e. CM would use this feature) or as a user-only feature?

I was thinking at both replacing the general and specific features and
letting users call it in case they want to add their own stuff.

> 
>>  6) don't provide any top-level exception for errors occurring in
>>     user-provided code (for solvers, ode, matrix visitors ...) but
>>     ask them in documentation to use their own unchecked exception
>>     that [math] will never see (i.e. remove FunctionEvaluationException,
>>     DerivativeException, MatrixVisitorException)
> 
> +1 for removing all exceptions for which there doesn't exist any "throw"
>    statement within CM. And also "MathUserException" (the few uses of it in
>    trunk should be replaced).
> 
>> I'm not sure for NullPointer/NullArgument. Perhaps our own
>> NullArgumentException with the [lang] exception context principle would
>> be fine. I doubt we should check everything by ourselves (it would be a
>> real performance killer), so whatever we choose there will be untracked
>> NPE. We should check some arguments where it makes sense, which is what
>> we already do at some places.
> 
> +1 for dropping "NullArgumentException" and letting the JVM raise NPE.
> 
>> Hoping to conclude this fast ...
> 
> Let's not be too hasty ;-). There can be some work left for 4.0.

I hope this would be algorithm work.

Luc

> 
> 
> Best 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