[Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

classic Classic list List threaded Threaded
35 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Michael Giannakopoulos
Hello guys,

As far as this issue is concerned (for what i have understood) i believe
that one way to separate NULL(s) that occur from the A.P.I. from NULL(s)
coming from wrong usage of A.P.I. by a user is the assert technique... I
didn't know a lot about it but from what i have read it should be
implemented only in the private methods of the A.P.I. Check this link out: "
http://download.oracle.com/javase/1.4.2/docs/guide/lang/assert.html".
Another choice is to create a new class that would check all the arguments
of every function we are interested in (for example: public
checkArguments(Object... args)) [If i have understood correctly the purpose
of this issue...]. Any suggestions would be more than welcomed!

Best regards,
Giannakopoulos Michael
Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Paul Benedict
Giannakopoulos,

It's a debate that goes on. Josh Bloch in his Effective Java book says NPE
is perfectly acceptable for bad arguments. So it really depends on your
perspective what an NPE represents. I prefer Josh's opinion but only because
every single argument probably creates lots of branch-checking that kills
cpu pipelining.

Paul

On Tue, Mar 1, 2011 at 2:01 PM, Michael Giannakopoulos <[hidden email]
> wrote:

> Hello guys,
>
> As far as this issue is concerned (for what i have understood) i believe
> that one way to separate NULL(s) that occur from the A.P.I. from NULL(s)
> coming from wrong usage of A.P.I. by a user is the assert technique... I
> didn't know a lot about it but from what i have read it should be
> implemented only in the private methods of the A.P.I. Check this link out:
> "
> http://download.oracle.com/javase/1.4.2/docs/guide/lang/assert.html".
> Another choice is to create a new class that would check all the arguments
> of every function we are interested in (for example: public
> checkArguments(Object... args)) [If i have understood correctly the purpose
> of this issue...]. Any suggestions would be more than welcomed!
>
> Best regards,
> Giannakopoulos Michael
>
Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Gilles Sadowski
Hi.

> It's a debate that goes on. Josh Bloch in his Effective Java book says NPE
> is perfectly acceptable for bad arguments. So it really depends on your
> perspective what an NPE represents. I prefer Josh's opinion but only because
> every single argument probably creates lots of branch-checking that kills
> cpu pipelining.
>
> > As far as this issue is concerned (for what i have understood) i believe
> > that one way to separate NULL(s) that occur from the A.P.I. from NULL(s)
> > coming from wrong usage of A.P.I. by a user is the assert technique... I
> > didn't know a lot about it but from what i have read it should be
> > implemented only in the private methods of the A.P.I. Check this link out:
> > "
> > http://download.oracle.com/javase/1.4.2/docs/guide/lang/assert.html".
> > Another choice is to create a new class that would check all the arguments
> > of every function we are interested in (for example: public
> > checkArguments(Object... args)) [If i have understood correctly the purpose
> > of this issue...]. Any suggestions would be more than welcomed!

NPE is the symptom of a bug.
Using "NullArgumentException" instead of the standard NPE so that the CM
exception hierarchy is singly rooted (the root being "MathRuntimeEception"
in the development version). An advantage is that it is easy to determine
whether an exception is generated by CM. A drawback is that it is
non-standard but this is mitigated by the fact that all other exceptions are
also non-standard (e.g. "MathIllegalArgumentException" instead of IAE).
One has to take into account that we settled on this choice because it makes
it somewhat easier to implement other requirements (namely the localization
of the error messages). It's a compromise (without the localization
requirement, I would have favoured the standard exceptions). And, apart from
avoiding code duplication, this choice has some "features" (which might be
construed as advantages or drawbacks, depending on the viewpoint)...

I'm not sure of what you mean by "branch-checking", but I don't think that
checking for null makes the problem bigger than it would be otherwise, since
CM already checks for many things.

In the end, I'm really not sure what is the best approach for this
particular case. Personally, I'd be happy that the CM code never checks for
null and let the JVM throw NPE. This would hugely simplify the CM code,
albeit at the cost of detecting bad usage a little later. IMHO, it is not a
big deal because the bug is that an object is missing somewhere up the call
stack, and it should be corrected there...

Of course, this would mean that MATH-403 should be dropped, the
"NullArgumentException" class removed, and the policy changed to: "Never
check for null references".


Best,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Bill Barker


-----Original Message-----
From: Gilles Sadowski
Sent: Tuesday, March 01, 2011 3:12 PM
To: [hidden email]
Subject: Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API

>Hi.
>
>> It's a debate that goes on. Josh Bloch in his Effective Java book says
>> NPE
>> is perfectly acceptable for bad arguments. So it really depends on your
>> perspective what an NPE represents. I prefer Josh's opinion but only
>> because
>> every single argument probably creates lots of branch-checking that kills
>> cpu pipelining.
>>
>> > As far as this issue is concerned (for what i have understood) i
>> > believe
>> > that one way to separate NULL(s) that occur from the A.P.I. from
>> > NULL(s)
>> > coming from wrong usage of A.P.I. by a user is the assert technique...
>> > I
>> > didn't know a lot about it but from what i have read it should be
>> > implemented only in the private methods of the A.P.I. Check this link
>> > out:
>> > "
>> > http://download.oracle.com/javase/1.4.2/docs/guide/lang/assert.html".
>> > Another choice is to create a new class that would check all the
>> > arguments
>> > of every function we are interested in (for example: public
>> > checkArguments(Object... args)) [If i have understood correctly the
>> > purpose
>> > of this issue...]. Any suggestions would be more than welcomed!
>
>NPE is the symptom of a bug.
>Using "NullArgumentException" instead of the standard NPE so that the CM
>exception hierarchy is singly rooted (the root being "MathRuntimeEception"
>in the development version). An advantage is that it is easy to determine
>whether an exception is generated by CM. A drawback is that it is
>non-standard but this is mitigated by the fact that all other exceptions
>are
>also non-standard (e.g. "MathIllegalArgumentException" instead of IAE).
>One has to take into account that we settled on this choice because it
>makes
>it somewhat easier to implement other requirements (namely the localization
>of the error messages). It's a compromise (without the localization
>requirement, I would have favoured the standard exceptions). And, apart
>from
>avoiding code duplication, this choice has some "features" (which might be
>construed as advantages or drawbacks, depending on the viewpoint)...
>
>I'm not sure of what you mean by "branch-checking", but I don't think that
>checking for null makes the problem bigger than it would be otherwise,
>since
>CM already checks for many things.
>
>In the end, I'm really not sure what is the best approach for this
>particular case. Personally, I'd be happy that the CM code never checks for
>null and let the JVM throw NPE. This would hugely simplify the CM code,
>albeit at the cost of detecting bad usage a little later. IMHO, it is not a
>big deal because the bug is that an object is missing somewhere up the call
>stack, and it should be corrected there...
>

I'm in favor of just letting the JVM throw NPE.  Since there is no message
in this case, there is nothing to localize.  Using a class to check
arguments is too much work, since the (localized) message "Something was
null" is less than helpful.  And assert will be turned off in any reasonably
configured production server so makes the code less readable and adds very
little value.  If the null happens because of code in CM (as opposed to user
error), then we'll get a Jira issue, fix it, and add a unit test.  If it is
user error, then the stack trace of the NPE will tell the developer what was
wrong in at least 95% of the cases.



>Of course, this would mean that MATH-403 should be dropped, the
>"NullArgumentException" class removed, and the policy changed to: "Never
>check for null references".
>
>
>Best,
>Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Paul Benedict
BTW, you can find precedence in the JVM for many methods that throw NPE on
null arguments. I am not saying this is the "right way", since such things
are subjective and are a matter of design, but many people have concluded
it's better.

On Tue, Mar 1, 2011 at 9:16 PM, Bill Barker <[hidden email]> wrote:

>
>
> -----Original Message----- From: Gilles Sadowski
> Sent: Tuesday, March 01, 2011 3:12 PM
> To: [hidden email]
> Subject: Re: [Math - 403] Never propagate a "NullPointerException"
> resulting from bad usage of the API
>
>
>  Hi.
>>
>>  It's a debate that goes on. Josh Bloch in his Effective Java book says
>>> NPE
>>> is perfectly acceptable for bad arguments. So it really depends on your
>>> perspective what an NPE represents. I prefer Josh's opinion but only
>>> because
>>> every single argument probably creates lots of branch-checking that kills
>>> cpu pipelining.
>>>
>>> > As far as this issue is concerned (for what i have understood) i >
>>> believe
>>> > that one way to separate NULL(s) that occur from the A.P.I. from >
>>> NULL(s)
>>> > coming from wrong usage of A.P.I. by a user is the assert technique...
>>> > I
>>> > didn't know a lot about it but from what i have read it should be
>>> > implemented only in the private methods of the A.P.I. Check this link >
>>> out:
>>> > "
>>> > http://download.oracle.com/javase/1.4.2/docs/guide/lang/assert.html".
>>> > Another choice is to create a new class that would check all the >
>>> arguments
>>> > of every function we are interested in (for example: public
>>> > checkArguments(Object... args)) [If i have understood correctly the >
>>> purpose
>>> > of this issue...]. Any suggestions would be more than welcomed!
>>>
>>
>> NPE is the symptom of a bug.
>> Using "NullArgumentException" instead of the standard NPE so that the CM
>> exception hierarchy is singly rooted (the root being "MathRuntimeEception"
>> in the development version). An advantage is that it is easy to determine
>> whether an exception is generated by CM. A drawback is that it is
>> non-standard but this is mitigated by the fact that all other exceptions
>> are
>> also non-standard (e.g. "MathIllegalArgumentException" instead of IAE).
>> One has to take into account that we settled on this choice because it
>> makes
>> it somewhat easier to implement other requirements (namely the
>> localization
>> of the error messages). It's a compromise (without the localization
>> requirement, I would have favoured the standard exceptions). And, apart
>> from
>> avoiding code duplication, this choice has some "features" (which might be
>> construed as advantages or drawbacks, depending on the viewpoint)...
>>
>> I'm not sure of what you mean by "branch-checking", but I don't think that
>> checking for null makes the problem bigger than it would be otherwise,
>> since
>> CM already checks for many things.
>>
>> In the end, I'm really not sure what is the best approach for this
>> particular case. Personally, I'd be happy that the CM code never checks
>> for
>> null and let the JVM throw NPE. This would hugely simplify the CM code,
>> albeit at the cost of detecting bad usage a little later. IMHO, it is not
>> a
>> big deal because the bug is that an object is missing somewhere up the
>> call
>> stack, and it should be corrected there...
>>
>>
> I'm in favor of just letting the JVM throw NPE.  Since there is no message
> in this case, there is nothing to localize.  Using a class to check
> arguments is too much work, since the (localized) message "Something was
> null" is less than helpful.  And assert will be turned off in any reasonably
> configured production server so makes the code less readable and adds very
> little value.  If the null happens because of code in CM (as opposed to user
> error), then we'll get a Jira issue, fix it, and add a unit test.  If it is
> user error, then the stack trace of the NPE will tell the developer what was
> wrong in at least 95% of the cases.
>
>
>
>
>  Of course, this would mean that MATH-403 should be dropped, the
>> "NullArgumentException" class removed, and the policy changed to: "Never
>> check for null references".
>>
>>
>> Best,
>> Gilles
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

jodastephen
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.

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).

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).

Stephen


On 2 March 2011 08:20, Paul Benedict <[hidden email]> wrote:

> BTW, you can find precedence in the JVM for many methods that throw NPE on
> null arguments. I am not saying this is the "right way", since such things
> are subjective and are a matter of design, but many people have concluded
> it's better.
>
> On Tue, Mar 1, 2011 at 9:16 PM, Bill Barker <[hidden email]> wrote:
>
>>
>>
>> -----Original Message----- From: Gilles Sadowski
>> Sent: Tuesday, March 01, 2011 3:12 PM
>> To: [hidden email]
>> Subject: Re: [Math - 403] Never propagate a "NullPointerException"
>> resulting from bad usage of the API
>>
>>
>>  Hi.
>>>
>>>  It's a debate that goes on. Josh Bloch in his Effective Java book says
>>>> NPE
>>>> is perfectly acceptable for bad arguments. So it really depends on your
>>>> perspective what an NPE represents. I prefer Josh's opinion but only
>>>> because
>>>> every single argument probably creates lots of branch-checking that kills
>>>> cpu pipelining.
>>>>
>>>> > As far as this issue is concerned (for what i have understood) i >
>>>> believe
>>>> > that one way to separate NULL(s) that occur from the A.P.I. from >
>>>> NULL(s)
>>>> > coming from wrong usage of A.P.I. by a user is the assert technique...
>>>> > I
>>>> > didn't know a lot about it but from what i have read it should be
>>>> > implemented only in the private methods of the A.P.I. Check this link >
>>>> out:
>>>> > "
>>>> > http://download.oracle.com/javase/1.4.2/docs/guide/lang/assert.html".
>>>> > Another choice is to create a new class that would check all the >
>>>> arguments
>>>> > of every function we are interested in (for example: public
>>>> > checkArguments(Object... args)) [If i have understood correctly the >
>>>> purpose
>>>> > of this issue...]. Any suggestions would be more than welcomed!
>>>>
>>>
>>> NPE is the symptom of a bug.
>>> Using "NullArgumentException" instead of the standard NPE so that the CM
>>> exception hierarchy is singly rooted (the root being "MathRuntimeEception"
>>> in the development version). An advantage is that it is easy to determine
>>> whether an exception is generated by CM. A drawback is that it is
>>> non-standard but this is mitigated by the fact that all other exceptions
>>> are
>>> also non-standard (e.g. "MathIllegalArgumentException" instead of IAE).
>>> One has to take into account that we settled on this choice because it
>>> makes
>>> it somewhat easier to implement other requirements (namely the
>>> localization
>>> of the error messages). It's a compromise (without the localization
>>> requirement, I would have favoured the standard exceptions). And, apart
>>> from
>>> avoiding code duplication, this choice has some "features" (which might be
>>> construed as advantages or drawbacks, depending on the viewpoint)...
>>>
>>> I'm not sure of what you mean by "branch-checking", but I don't think that
>>> checking for null makes the problem bigger than it would be otherwise,
>>> since
>>> CM already checks for many things.
>>>
>>> In the end, I'm really not sure what is the best approach for this
>>> particular case. Personally, I'd be happy that the CM code never checks
>>> for
>>> null and let the JVM throw NPE. This would hugely simplify the CM code,
>>> albeit at the cost of detecting bad usage a little later. IMHO, it is not
>>> a
>>> big deal because the bug is that an object is missing somewhere up the
>>> call
>>> stack, and it should be corrected there...
>>>
>>>
>> I'm in favor of just letting the JVM throw NPE.  Since there is no message
>> in this case, there is nothing to localize.  Using a class to check
>> arguments is too much work, since the (localized) message "Something was
>> null" is less than helpful.  And assert will be turned off in any reasonably
>> configured production server so makes the code less readable and adds very
>> little value.  If the null happens because of code in CM (as opposed to user
>> error), then we'll get a Jira issue, fix it, and add a unit test.  If it is
>> user error, then the stack trace of the NPE will tell the developer what was
>> wrong in at least 95% of the cases.
>>
>>
>>
>>
>>  Of course, this would mean that MATH-403 should be dropped, the
>>> "NullArgumentException" class removed, and the policy changed to: "Never
>>> check for null references".
>>>
>>>
>>> Best,
>>> Gilles
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Gilles Sadowski
In reply to this post by Bill Barker
> > [...]
>
> I'm in favor of just letting the JVM throw NPE.  Since there is no
> message in this case, there is nothing to localize.  Using a class
> to check arguments is too much work, since the (localized) message
> "Something was null" is less than helpful.  And assert will be
> turned off in any reasonably configured production server so makes
> the code less readable and adds very little value.  If the null
> happens because of code in CM (as opposed to user error), then we'll
> get a Jira issue, fix it, and add a unit test.  If it is user error,
> then the stack trace of the NPE will tell the developer what was
> wrong in at least 95% of the cases.

That was my first line of arguments, a long time ago. But there was no
agreement mainly because of the will to retain the possibility of localizing
the message, even for NPE (i.e. one should be able to supply a "Localizable"
argument to the constructor of "NullArgumentException"). A second
counter-argument was that in a certain use-case, no stack trace is
available.


Best,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Gilles Sadowski
In reply to this post by jodastephen
> 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).
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).

> 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. 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. 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).


Regards,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

sebb-2-2
In reply to this post by Paul Benedict
On 2 March 2011 08:20, Paul Benedict <[hidden email]> wrote:
> BTW, you can find precedence in the JVM for many methods that throw NPE on
> null arguments. I am not saying this is the "right way", since such things
> are subjective and are a matter of design, but many people have concluded
> it's better.

If the NPE would not be detected until the method has done some other
work, then I can seem why one might want to detect it earlier.

And the line number may be insufficient to identify the source of the
NPE - there could be several de-references in a single line.

> On Tue, Mar 1, 2011 at 9:16 PM, Bill Barker <[hidden email]> wrote:
>
>>
>>
>> -----Original Message----- From: Gilles Sadowski
>> Sent: Tuesday, March 01, 2011 3:12 PM
>> To: [hidden email]
>> Subject: Re: [Math - 403] Never propagate a "NullPointerException"
>> resulting from bad usage of the API
>>
>>
>>  Hi.
>>>
>>>  It's a debate that goes on. Josh Bloch in his Effective Java book says
>>>> NPE
>>>> is perfectly acceptable for bad arguments. So it really depends on your
>>>> perspective what an NPE represents. I prefer Josh's opinion but only
>>>> because
>>>> every single argument probably creates lots of branch-checking that kills
>>>> cpu pipelining.
>>>>
>>>> > As far as this issue is concerned (for what i have understood) i >
>>>> believe
>>>> > that one way to separate NULL(s) that occur from the A.P.I. from >
>>>> NULL(s)
>>>> > coming from wrong usage of A.P.I. by a user is the assert technique...
>>>> > I
>>>> > didn't know a lot about it but from what i have read it should be
>>>> > implemented only in the private methods of the A.P.I. Check this link >
>>>> out:
>>>> > "
>>>> > http://download.oracle.com/javase/1.4.2/docs/guide/lang/assert.html".
>>>> > Another choice is to create a new class that would check all the >
>>>> arguments
>>>> > of every function we are interested in (for example: public
>>>> > checkArguments(Object... args)) [If i have understood correctly the >
>>>> purpose
>>>> > of this issue...]. Any suggestions would be more than welcomed!
>>>>
>>>
>>> NPE is the symptom of a bug.
>>> Using "NullArgumentException" instead of the standard NPE so that the CM
>>> exception hierarchy is singly rooted (the root being "MathRuntimeEception"
>>> in the development version). An advantage is that it is easy to determine
>>> whether an exception is generated by CM. A drawback is that it is
>>> non-standard but this is mitigated by the fact that all other exceptions
>>> are
>>> also non-standard (e.g. "MathIllegalArgumentException" instead of IAE).
>>> One has to take into account that we settled on this choice because it
>>> makes
>>> it somewhat easier to implement other requirements (namely the
>>> localization
>>> of the error messages). It's a compromise (without the localization
>>> requirement, I would have favoured the standard exceptions). And, apart
>>> from
>>> avoiding code duplication, this choice has some "features" (which might be
>>> construed as advantages or drawbacks, depending on the viewpoint)...
>>>
>>> I'm not sure of what you mean by "branch-checking", but I don't think that
>>> checking for null makes the problem bigger than it would be otherwise,
>>> since
>>> CM already checks for many things.
>>>
>>> In the end, I'm really not sure what is the best approach for this
>>> particular case. Personally, I'd be happy that the CM code never checks
>>> for
>>> null and let the JVM throw NPE. This would hugely simplify the CM code,
>>> albeit at the cost of detecting bad usage a little later. IMHO, it is not
>>> a
>>> big deal because the bug is that an object is missing somewhere up the
>>> call
>>> stack, and it should be corrected there...
>>>
>>>
>> I'm in favor of just letting the JVM throw NPE.  Since there is no message
>> in this case, there is nothing to localize.  Using a class to check
>> arguments is too much work, since the (localized) message "Something was
>> null" is less than helpful.  And assert will be turned off in any reasonably
>> configured production server so makes the code less readable and adds very
>> little value.  If the null happens because of code in CM (as opposed to user
>> error), then we'll get a Jira issue, fix it, and add a unit test.  If it is
>> user error, then the stack trace of the NPE will tell the developer what was
>> wrong in at least 95% of the cases.
>>
>>
>>
>>
>>  Of course, this would mean that MATH-403 should be dropped, the
>>> "NullArgumentException" class removed, and the policy changed to: "Never
>>> check for null references".
>>>
>>>
>>> Best,
>>> Gilles
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Gilles Sadowski
> > BTW, you can find precedence in the JVM for many methods that throw NPE on
> > null arguments. I am not saying this is the "right way", since such things
> > are subjective and are a matter of design, but many people have concluded
> > it's better.
>
> If the NPE would not be detected until the method has done some other
> work, then I can seem why one might want to detect it earlier.
>
> And the line number may be insufficient to identify the source of the
> NPE - there could be several de-references in a single line.

This is the trade-off which I had mentioned here:

> >>> In the end, I'm really not sure what is the best approach for this
> >>> particular case. Personally, I'd be happy that the CM code never checks
> >>> for
> >>> null and let the JVM throw NPE. This would hugely simplify the CM code,
> >>> albeit at the cost of detecting bad usage a little later. IMHO, it is not
> >>> a
> >>> big deal because the bug is that an object is missing somewhere up the
> >>> call
> >>> stack, and it should be corrected there...


Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

sebb-2-2
On 2 March 2011 12:50, Gilles Sadowski <[hidden email]> wrote:
>> > BTW, you can find precedence in the JVM for many methods that throw NPE on
>> > null arguments. I am not saying this is the "right way", since such things
>> > are subjective and are a matter of design, but many people have concluded
>> > it's better.
>>
>> If the NPE would not be detected until the method has done some other
>> work, then I can seem why one might want to detect it earlier.

The above is the trade-off.

>> And the line number may be insufficient to identify the source of the
>> NPE - there could be several de-references in a single line.
>
> This is the trade-off which I had mentioned here:

Not really ...

>> >>> In the end, I'm really not sure what is the best approach for this
>> >>> particular case. Personally, I'd be happy that the CM code never checks
>> >>> for
>> >>> null and let the JVM throw NPE. This would hugely simplify the CM code,
>> >>> albeit at the cost of detecting bad usage a little later. IMHO, it is not
>> >>> a
>> >>> big deal because the bug is that an object is missing somewhere up the
>> >>> call
>> >>> stack, and it should be corrected there...
>

.. it may be impossible to determine the true cause of the NPE in some cases.

So in some cases, it makes sense to check for NPE at the start.

> Gilles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Luc Maisonobe
In reply to this post by Gilles Sadowski
Le 02/03/2011 12:37, Gilles Sadowski a écrit :
>> 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.

>
>> 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. So I would say localization and exception hierarchy are
completely orthogonal features.

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

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)
 2) keep localization
 3) don't put all exceptions in a dedicated exception package
    (but the general exceptions used everywhere could go there)
 4) put specific exceptions in the package they are triggered
    (for example ODE, linear ...)
 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
 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)

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.

Hoping to conclude this fast ...
Luc

>
>
> Regards,
> Gilles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Gilles Sadowski
In reply to this post by sebb-2-2
On Wed, Mar 02, 2011 at 01:11:48PM +0000, sebb wrote:

> On 2 March 2011 12:50, Gilles Sadowski <[hidden email]> wrote:
> >> > BTW, you can find precedence in the JVM for many methods that throw NPE on
> >> > null arguments. I am not saying this is the "right way", since such things
> >> > are subjective and are a matter of design, but many people have concluded
> >> > it's better.
> >>
> >> If the NPE would not be detected until the method has done some other
> >> work, then I can seem why one might want to detect it earlier.
>
> The above is the trade-off.

Hence you say that the programmer can choose to test or not. Indeed
this is already how it is done in CM: Sometimes there is a check and
sometimes not. So we'd have to state a guideline such that
 "If the NPE would not be detected until the method has done some other
  work, checks for null references should be done at start."
And also that, if the reference is null, a (standard) NPE must be thrown
(without a message, because it wouldn't be localized).

But how much work warrants an additional check?
What if there is no "other work" in the original code but someone adds some
later on (and forget to add the check)?
Overall, I'd think that the advantage is not worth the complication.

> >> And the line number may be insufficient to identify the source of the
> >> NPE - there could be several de-references in a single line.
> >
> > This is the trade-off which I had mentioned here:
>
> Not really ...
>
> >> >>> In the end, I'm really not sure what is the best approach for this
> >> >>> particular case. Personally, I'd be happy that the CM code never checks
> >> >>> for
> >> >>> null and let the JVM throw NPE. This would hugely simplify the CM code,
> >> >>> albeit at the cost of detecting bad usage a little later. IMHO, it is not
> >> >>> a
> >> >>> big deal because the bug is that an object is missing somewhere up the
> >> >>> call
> >> >>> stack, and it should be corrected there...
> >
>
> .. it may be impossible to determine the true cause of the NPE in some cases.

Even when re-writing the statements on multiple lines for debugging
purposes?

> So in some cases, it makes sense to check for NPE at the start.

I didn't say otherwise.
But we might want to trade-off the possibility of _easily_ spotting the
failure (this is what I meant by "at the cost of detecting bad usage a
little later") for improved performance.

I still favour the simplest option: no checks for null whatsoever.


Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Phil Steitz
In reply to this post by Luc Maisonobe
On 3/2/11 8:32 AM, Luc Maisonobe wrote:

> Le 02/03/2011 12:37, Gilles Sadowski a écrit :
>>> 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.
>
>>> 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. So I would say localization and exception hierarchy are
> completely orthogonal features.
>
>> 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 ?
>
> 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)
+0 - move on
>  2) keep localization
+1
>  3) don't put all exceptions in a dedicated exception package
>     (but the general exceptions used everywhere could go there)
+1
>  4) put specific exceptions in the package they are triggered
>     (for example ODE, linear ...)
+1
>  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
Not sure exactly what is meant here.  I don't think it is best to
devolve all exception context into dynamic name/value pairs.  I It
is probably best to look at examples and base our decision on what
is easiest to manage and understand.  I am fine adding this to the
arsenal and allowing users to use it to add more stuff to
exceptions, but I need to see how it would work in examples before
embarking on wholesale replacement.

Regarding the hierarchy, what I would like is a shallow but
meaningful hierarchy of exception classes that encapsulate the kinds
of things that fail in [math].  We have been talking about a few of
them - iterative algorithms fail to converge, attempts at evaluating
a function fail, input data or problem parameters violate
preconditions, irrecoverable numerical problems occur...
It would be great to think about these both at the top [math] level
and in relation to the specific packages.  Could be things are fine
the way they are (trunk) or were (2.1).  In any case, we need to
decide what the hierarchy looks like.
>  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)
I agree with adding the concept of MathUserException, but I suspect
there are internal uses and independent value in
FunctionEvaluationException, DerivativeException and
MatrixVisitorException.  If the only uses we can ever see of these
is for user code, then OK; but IIRC, we do use
FunctionEvaluationException and DerivativeException internally.  Is
there a way that we can have it both ways?  Still maintain these
abstractions for use in [math] and by applications that use [math];
but still allow users to effectively propagate their own exceptions
through the [math] API layers?
> 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.
I agree that by and large, the null-checking in place now is
adequate and going overboard will lead to performance issues.  I
also agree with what others have said that to some extent NPE is
life in Java, so move on.  I did prefer the 2.1 approach to creating
RTEs, though.

> Hoping to conclude this fast ...
> Luc
>
>>
>> Regards,
>> Gilles
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Adrian Crum-3
In reply to this post by Gilles Sadowski
I agree with this view. It would help the developer who uses CM if the
library told him/her what they did wrong ("argument 'foo' cannot be
null") instead of a simple exception thrown message
("NullPointerException thrown at line nnn of class Xyz").

-Adrian


On 3/2/2011 3:37 AM, Gilles Sadowski wrote:
> 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).
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Paul Benedict
I neglected to mention that Commons Math *should* document what exceptions
are to be expected. If a method is designed to throw NPE because of a null
argument (whether through explicit checking or not), @throws should mention
that.

Paul

On Wed, Mar 2, 2011 at 10:36 AM, Adrian Crum <
[hidden email]> wrote:

> I agree with this view. It would help the developer who uses CM if the
> library told him/her what they did wrong ("argument 'foo' cannot be null")
> instead of a simple exception thrown message ("NullPointerException thrown
> at line nnn of class Xyz").
>
> -Adrian
>
>
> On 3/2/2011 3:37 AM, Gilles Sadowski wrote:
>
>> 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).
>>
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

jodastephen
I consider it sufficient (preferable) to do the following:

@param foo the foo, not null

Where the ", not null" implies NPE when a null is passed in (document
once in the overview javadoc if you feel necessary). This approach is
easier to transfer to a @NotNull annotation in the future (or the
proper language solution of nullable types!)

Stephen


On 2 March 2011 16:50, Paul Benedict <[hidden email]> wrote:

> I neglected to mention that Commons Math *should* document what exceptions
> are to be expected. If a method is designed to throw NPE because of a null
> argument (whether through explicit checking or not), @throws should mention
> that.
>
> Paul
>
> On Wed, Mar 2, 2011 at 10:36 AM, Adrian Crum <
> [hidden email]> wrote:
>
>> I agree with this view. It would help the developer who uses CM if the
>> library told him/her what they did wrong ("argument 'foo' cannot be null")
>> instead of a simple exception thrown message ("NullPointerException thrown
>> at line nnn of class Xyz").
>>
>> -Adrian
>>
>>
>> On 3/2/2011 3:37 AM, Gilles Sadowski wrote:
>>
>>> 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).
>>>
>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Gilles Sadowski
In reply to this post by Luc Maisonobe
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).

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

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

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 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).

>  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).

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

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


Best regards,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Michael Giannakopoulos
I believe that the majority of exceptions should be in the same package and
not to under different ones...


> >>because most exceptions are general and thus I don't see the benefit of
> >>scattering the remaining few among several packages.
>

+1 for the proposal of Gilles


>  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 the one of Luc


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

+1 to this one

Also i think that we should check for some exceptions that are not covered
by those which have been implemented so far (for example when we have two
numbers 'a' and 'b' and 'a' should always be greater than 'b'...). In this
case we should implement them... And of course, to erase all those
exceptions for which no 'throw' clause exists as Gilles said.

Friendly,
Giannakopoulos Michael
Reply | Threaded
Open this post in threaded view
|

Re: [Math - 403] Never propagate a "NullPointerException" resulting from bad usage of the API

Luc Maisonobe
In reply to this post by Gilles Sadowski
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: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

12