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 - 403] Never propagate a "NullPointerException" resulting from bad usage of the API
Date Fri, 04 Mar 2011 19:49:00 GMT
On 3/3/11 5:51 PM, Gilles Sadowski wrote:
> Hi.
>
>>>> 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.
>>> Yes, the "...MatrixException" classes are the borderline case (meaning that,
>>> if they were the majority of exception classes, I'd prefer to see them in
>>> the "linear" package). However, I argue that when there exists an
>>> "exception" to contain the general exceptions, then it is clearer to have
>>> them all there.
>>> Moreover "NonPositiveDefiniteMatrixException" is already a counter-example
>>> as it is used in "linear" and in "random".
>> It is used in random because the classes in the random package do use
>> the linear package to perform a Cholesky decomposition, which triggers
>> the exception. This is an example of the last sentence in my rationale
>> above. This appearance is not sufficient to put the exception out of linear.
> The class "CorrelatedRandomVectorGenerator" does not use the
> "CholeskyDecomposition" from package "linear"; it has its own "decompose"
> method which explicitly throws "NonPositiveDefiniteMatrixException"
> instances (at lines 214 and 222).
>
>>> Since we probably won't have much more exception classes than there are now
>>> (31 out of which less than 10 are probably candidates for relocation in
>>> their main use package), it is fairly manageable and it will be more stable.
>> Let's wait for the opinions of other people and decide upon that.
>>
>>>>>>  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.
>>> I'll create a JIRA issue for the new "map" feature, to discuss the details
>>> of the functionality.
>> Fine, but discussion is easier on the list (with a dedicated thread), we
>> can open the Jira issue once we have decided what to do.
> I propose to extend the "MathThrowable" interface with methods declarations
> similar to what is done in [Lang] and implement the functionality in
> "MathRuntimeException". [While looking at it, I notice that [Lang] has
> a dedicated "exception" package...]
>
>>>>>>  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.
>>> I'll also create a JIRA issue for reaching a conclusion on this one.
>> OK, I think we agree here. Phil preferred the way we created runtime
>> exceptions as of 2.1, though. Perhaps we could revive a simple
>> createNullPointerException without arguments ?
> But why would one prefer to write
> ---
>   throw MathRuntimeException.createNullPointerException();
> ---
> instead of
> ---
>   throw new NullPointerException();
> ---
> ?
>
> Not only is this inconvenient, it is also dangerous: I was bitten more than
> once writing this:
> ---
>   MathRuntimeException.createNullPointerException();
> ---
> which the compiler happily accepted.
I agree that it would be nicer to just create a standard exception
with new.  What I liked about the 2.1 setup, though, was that while
I admit it was a little ugly to say
MatRuntimeException.createIllegalArgumentException(..parameters..),
the result was a standard IAE (with a powerful message formatting
and localization engine to create the message).  In trunk, we now
get to say "new" but we are forced into nonstandard exceptions.

I agree that the root cause of the inconvenience here is the
localization.  Personally, I was fine with the 2.1 setup and prefer
it to what is in trunk; but I think it is more important for us to
move on so am fine leaving it as is.  I would like us to keep both
localization and the flexible message formatting that we have had
since 1.2.

Phil


>>>>>> 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.
>>> New algorithms can always be included in 3.x releases. I was referring to
>>> a "refactoring" (which entails incompatibilities). This is not a bad thing;
>>> quite the contrary, this is how a programming project stays alive, in sync
>>> with technology advances, and keeps attracting developers.
>> Yes, but lengthy and harsh discussions keep them away.
> Indeed, indeed...
>
>
> 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


Mime
View raw message