You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Michael Giannakopoulos <mi...@gmail.com> on 2011/03/01 21:01:49 UTC
[Math - 403] Never propagate a "NullPointerException" resulting from
bad usage of the API
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
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Wed, Mar 02, 2011 at 01:11:48PM +0000, sebb wrote:
> On 2 March 2011 12:50, Gilles Sadowski <gi...@harfang.homelinux.org> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by sebb <se...@gmail.com>.
On 2 March 2011 12:50, Gilles Sadowski <gi...@harfang.homelinux.org> 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: 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
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> > 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by sebb <se...@gmail.com>.
On 2 March 2011 08:20, Paul Benedict <pb...@apache.org> 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 <bi...@verizon.net> wrote:
>
>>
>>
>> -----Original Message----- From: Gilles Sadowski
>> Sent: Tuesday, March 01, 2011 3:12 PM
>> To: dev@commons.apache.org
>> 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: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Ole Ersoy <ol...@gmail.com>.
Sure - For example the LaguerreSolver has the following set of lines in the start of the solveAll method:
if (coefficients == null) {
throw new NullArgumentException();
}
int n = coefficients.length - 1;
if (n == 0) {
throw new NoDataException(LocalizedFormats.POLYNOMIAL);
}
Move these to a Validator class (Could be an inner class of the LaguerreSolver) that has a validateSolveAll method in it:
public void validateSolveAll(Complex coefficients[], Complex initial)
{
if (coefficients == null) {
throw new NullArgumentException();
}
int n = coefficients.length - 1;
if (n == 0) {
throw new NoDataException(LocalizedFormats.POLYNOMIAL);
}
}
Now if you are the client you would do:
validateSolveAll(..)
solveAll(..)
If you are a user interface designer you would wrap the validateSolveAll in a validator for the user interface framework being used to provide real time feedback to the user. It makes life easier for the GUI Designer because the validation exceptions have been separated from other types of exceptions, like a ConvergenceException.
Cheers,
- Ole
On 03/04/2011 05:32 PM, Gilles Sadowski wrote:
> Hi.
>
>> Just want to throw another idea into the mix. How about stripping all the validation argument validation lines and putting in separate validator classes. Clients would call the validator on the arguments for the class before passing the arguments to the class's method. It makes the core classes more focused and efficient and enables clients to wrap method invocations with aspects that apply the validation routine.
>
> Could you post a little code example?
>
> 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
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.
> Just want to throw another idea into the mix. How about stripping all the validation argument validation lines and putting in separate validator classes. Clients would call the validator on the arguments for the class before passing the arguments to the class's method. It makes the core classes more focused and efficient and enables clients to wrap method invocations with aspects that apply the validation routine.
Could you post a little code example?
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Luc Maisonobe <Lu...@free.fr>.
Le 06/03/2011 14:31, Gilles Sadowski a écrit :
> Hello Luc.
Hi,
>
>>>
>>>> [...]
>>>>> So, what is the lesson from having this case? Is the whole method a duplicate
>>>>> that should be replace by a call to something in "linear"? Or is it a proof
>>>>> that some exceptions might be appropriately used outside of "linear"?
>>>>
>>>> I think the method should be moved to a new
>>>> TruncatedCholeskyDecomposition or RectangularCholeskyDecomposition. I
>>>> don't think we can add it has a new implementation or it should be added
>>>> to the existing CholeskyDecomposition because the decomposition matrix
>>>> shape is not triangular.
>>>
>>> If you are going to create new classes in "linear" so that the current CM
>>> code only throws "...MatrixException"s from inside that package, I'm not
>>> going argue anymore about moving those exceptions to "linear". But, if this
>>> situation arises again, then we will create a incompatibility by moving the
>>> exception from "linear" to "exception"... That's why I think that it's safer
>>> (from a stability point-of-view) to have them all in "exception".
>>
>> I don't think this will happen very often.
>> So let's create the new classes (I'll open a Jira issue for that) and
>> move the linear algebra related exceptions back to the linear package.
>
> I'll do the move. Please, don't touch anything in the "exception" package at
> the moment: I'm currently implementing the "map" change.
OK, I'll wait until you have implemented the map feature.
>
>>>>>>> [...]
>>>>>>
>>>>>> For example to have a single point where to put breakpoints for
>>>>>> debugging. I used that a lot as such exceptions mainly occur during
>>>>>> development phase.
>>>>>
>>>>> Thus, IIUC, this is a developer feature.
>>>>> I understand the convenience, but I wouldn't trade code consistency for it.
>>>>> I'd rather keep "NullArgumentException" (or a "MathNullPointerException" if
>>>>> we want it to extend the standard NPE) so that you can put your breakpoint
>>>>> in the constructor of that exception. Thus you have the convenience without
>>>>> the code asymmetry.
>>>>
>>>> OK for MathNullPointerException.
>>>
>>> With a "checkNotNull" function, it should not be necessary.
>>> But I didn't quite understood your reasoning concerning such a method and
>>> whether it's OK to implement or not (cf. below).
>>>
>>>>>
>>>>>>>
>>>>>>> Not only is this inconvenient, it is also dangerous: I was bitten more than
>>>>>>> once writing this:
>>>>>>> ---
>>>>>>> MathRuntimeException.createNullPointerException();
>>>>>>> ---
>>>>>>> which the compiler happily accepted.
>>>>>>
>>>>>> Of course, as it would have accepted:
>>>>>>
>>>>>> new NullPointerException();
>>>>>>
>>>>>> Unfortunately, we cannot put the throw inside the method. Trying to do
>>>>>> this prevent the optimizer to wor properly as it does not know in the
>>>>>> caller that the method would never return, and it brings lots of
>>>>>> warnings from code checking tools for the same reason.
>>>>>
>>>>> The method alternative looses much of its appeal because it doesn't perform
>>>>> the complete action ("throw" included), as I had initially assumed when
>>>>> wrongly leaving out the "throw" statement. I wouldn't have written
>>>>> new NullPointerException();
>>>>> as, to me, it would have _obviously_ looked wrong.
>>>>>
>>>>> I don't understand the problem with the "optimizer". There are several
>>>>> methods in CM that seem to perform similarly (i.e. they check something and
>>>>> throw an exception). An example is "checkOrder" in "MathUtils".
>>>>> In fact we could add a "checkNotNull" in "MathUtils":
>>>>> ---
>>>>> public checkNotNull(Object obj) {
>>>>> if (obj == null) {
>>>>> throw new NullPointerException();
>>>>> }
>>>>> }
>>>>> ---
>>>>> Wouldn't that satisfy all needs (ability to set a breakpoint and no factory
>>>>> methods)?
>>>>
>>>> No the check may succeed or not, so sometimes these methods do return.
>>>
>>> I'd say that they return most of the time, and that's fortunate... :-)
>>
>> Sorry, I misunderstood what you were writing. I was still talking about
>> the changing the createXxxException (which only creates) into
>> throwXxxException (which creates and throws), not about
>> checkXxxCondition (which checks, creates and throws).
>>
>>>
>>>> What I had in mind was something like:
>>>>
>>>> public static void throwNPE() {
>>>> throw new MySpecialNPE extends NullPointerException() {
>>>> // my special code here
>>>> };
>>>> }
>>>>
>>>> Such a method *never* returns but on the caller side, the compiler, and
>>>> the checking tools don't know it. Typically when you have code like:
>>>>
>>>> int var;
>>>> if (obj != null) {
>>>> var = obj.getVar();
>>>> } else {
>>>> throwNPE();
>>>> }
>>>>
>>>> // use var
>>>>
>>>> Then tools will complain that var may not be initialized, which is
>>>> false. However it is only because throwNPE never returns that it is false.
>>>
>>> Personally, I don't like this kind of syntax. The good programming rule is
>>> to assign at declaration. [Unfortunately, one is sometimes forced to write
>>> in this way and most often than not because of checked exceptions!]
>>>
>>>> Another way to put it that would probably work is:
>>>>
>>>> public static void checkNonNull(Object o) {
>>>> if (o == null) {
>>>> throw new MySpecialNPE extends NullPointerException() {
>>>> // my special code here
>>>> };
>>>> }
>>>> }
>>>
>>> How is this different from my version above?
>>
>> It's the same, I simply get confused. So go for it, but including the
>> specialization and localization part that was in the former
>> createxxxException. So basically we add an if (obj == null) wrapper
>> aroud the 2.1 code.
>
> So to be sure we say the same thing, you'd like to have (in "MathUtils"):
> ---
> public static void checkNotNull(Object o,
> Localizable pattern,
> Object ... args) {
> if (o == null) {
> throw new NullArgumentException(pattern, args);
> }
> }
> ---
> Where we keep the _non-standard_ "NullArgumentException". [In line with what
> we agreed on before: That it made sense (for the various reasons we dicussed
> at length) to have a singly rooted hierarchy and thus depart from the
> standard exceptions sub-classes.]
>
> And I'll also add
> ---
> public static void checkNotNull(Object o) {
> if (o == null) {
> throw new NullArgumentException();
> }
> }
> ---
> for those developers who will be content with the default message ("null is
> not allowed").
>
> OK?
Yes.
best regards,
Luc
>
>
> 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
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hello Luc.
> >
> >> [...]
> >>> So, what is the lesson from having this case? Is the whole method a duplicate
> >>> that should be replace by a call to something in "linear"? Or is it a proof
> >>> that some exceptions might be appropriately used outside of "linear"?
> >>
> >> I think the method should be moved to a new
> >> TruncatedCholeskyDecomposition or RectangularCholeskyDecomposition. I
> >> don't think we can add it has a new implementation or it should be added
> >> to the existing CholeskyDecomposition because the decomposition matrix
> >> shape is not triangular.
> >
> > If you are going to create new classes in "linear" so that the current CM
> > code only throws "...MatrixException"s from inside that package, I'm not
> > going argue anymore about moving those exceptions to "linear". But, if this
> > situation arises again, then we will create a incompatibility by moving the
> > exception from "linear" to "exception"... That's why I think that it's safer
> > (from a stability point-of-view) to have them all in "exception".
>
> I don't think this will happen very often.
> So let's create the new classes (I'll open a Jira issue for that) and
> move the linear algebra related exceptions back to the linear package.
I'll do the move. Please, don't touch anything in the "exception" package at
the moment: I'm currently implementing the "map" change.
> >>>>> [...]
> >>>>
> >>>> For example to have a single point where to put breakpoints for
> >>>> debugging. I used that a lot as such exceptions mainly occur during
> >>>> development phase.
> >>>
> >>> Thus, IIUC, this is a developer feature.
> >>> I understand the convenience, but I wouldn't trade code consistency for it.
> >>> I'd rather keep "NullArgumentException" (or a "MathNullPointerException" if
> >>> we want it to extend the standard NPE) so that you can put your breakpoint
> >>> in the constructor of that exception. Thus you have the convenience without
> >>> the code asymmetry.
> >>
> >> OK for MathNullPointerException.
> >
> > With a "checkNotNull" function, it should not be necessary.
> > But I didn't quite understood your reasoning concerning such a method and
> > whether it's OK to implement or not (cf. below).
> >
> >>>
> >>>>>
> >>>>> Not only is this inconvenient, it is also dangerous: I was bitten more than
> >>>>> once writing this:
> >>>>> ---
> >>>>> MathRuntimeException.createNullPointerException();
> >>>>> ---
> >>>>> which the compiler happily accepted.
> >>>>
> >>>> Of course, as it would have accepted:
> >>>>
> >>>> new NullPointerException();
> >>>>
> >>>> Unfortunately, we cannot put the throw inside the method. Trying to do
> >>>> this prevent the optimizer to wor properly as it does not know in the
> >>>> caller that the method would never return, and it brings lots of
> >>>> warnings from code checking tools for the same reason.
> >>>
> >>> The method alternative looses much of its appeal because it doesn't perform
> >>> the complete action ("throw" included), as I had initially assumed when
> >>> wrongly leaving out the "throw" statement. I wouldn't have written
> >>> new NullPointerException();
> >>> as, to me, it would have _obviously_ looked wrong.
> >>>
> >>> I don't understand the problem with the "optimizer". There are several
> >>> methods in CM that seem to perform similarly (i.e. they check something and
> >>> throw an exception). An example is "checkOrder" in "MathUtils".
> >>> In fact we could add a "checkNotNull" in "MathUtils":
> >>> ---
> >>> public checkNotNull(Object obj) {
> >>> if (obj == null) {
> >>> throw new NullPointerException();
> >>> }
> >>> }
> >>> ---
> >>> Wouldn't that satisfy all needs (ability to set a breakpoint and no factory
> >>> methods)?
> >>
> >> No the check may succeed or not, so sometimes these methods do return.
> >
> > I'd say that they return most of the time, and that's fortunate... :-)
>
> Sorry, I misunderstood what you were writing. I was still talking about
> the changing the createXxxException (which only creates) into
> throwXxxException (which creates and throws), not about
> checkXxxCondition (which checks, creates and throws).
>
> >
> >> What I had in mind was something like:
> >>
> >> public static void throwNPE() {
> >> throw new MySpecialNPE extends NullPointerException() {
> >> // my special code here
> >> };
> >> }
> >>
> >> Such a method *never* returns but on the caller side, the compiler, and
> >> the checking tools don't know it. Typically when you have code like:
> >>
> >> int var;
> >> if (obj != null) {
> >> var = obj.getVar();
> >> } else {
> >> throwNPE();
> >> }
> >>
> >> // use var
> >>
> >> Then tools will complain that var may not be initialized, which is
> >> false. However it is only because throwNPE never returns that it is false.
> >
> > Personally, I don't like this kind of syntax. The good programming rule is
> > to assign at declaration. [Unfortunately, one is sometimes forced to write
> > in this way and most often than not because of checked exceptions!]
> >
> >> Another way to put it that would probably work is:
> >>
> >> public static void checkNonNull(Object o) {
> >> if (o == null) {
> >> throw new MySpecialNPE extends NullPointerException() {
> >> // my special code here
> >> };
> >> }
> >> }
> >
> > How is this different from my version above?
>
> It's the same, I simply get confused. So go for it, but including the
> specialization and localization part that was in the former
> createxxxException. So basically we add an if (obj == null) wrapper
> aroud the 2.1 code.
So to be sure we say the same thing, you'd like to have (in "MathUtils"):
---
public static void checkNotNull(Object o,
Localizable pattern,
Object ... args) {
if (o == null) {
throw new NullArgumentException(pattern, args);
}
}
---
Where we keep the _non-standard_ "NullArgumentException". [In line with what
we agreed on before: That it made sense (for the various reasons we dicussed
at length) to have a singly rooted hierarchy and thus depart from the
standard exceptions sub-classes.]
And I'll also add
---
public static void checkNotNull(Object o) {
if (o == null) {
throw new NullArgumentException();
}
}
---
for those developers who will be content with the default message ("null is
not allowed").
OK?
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Luc Maisonobe <Lu...@free.fr>.
Le 05/03/2011 00:59, Gilles Sadowski a écrit :
> Hello.
Hi Gilles,
>
>> [...]
>>> So, what is the lesson from having this case? Is the whole method a duplicate
>>> that should be replace by a call to something in "linear"? Or is it a proof
>>> that some exceptions might be appropriately used outside of "linear"?
>>
>> I think the method should be moved to a new
>> TruncatedCholeskyDecomposition or RectangularCholeskyDecomposition. I
>> don't think we can add it has a new implementation or it should be added
>> to the existing CholeskyDecomposition because the decomposition matrix
>> shape is not triangular.
>
> If you are going to create new classes in "linear" so that the current CM
> code only throws "...MatrixException"s from inside that package, I'm not
> going argue anymore about moving those exceptions to "linear". But, if this
> situation arises again, then we will create a incompatibility by moving the
> exception from "linear" to "exception"... That's why I think that it's safer
> (from a stability point-of-view) to have them all in "exception".
I don't think this will happen very often.
So let's create the new classes (I'll open a Jira issue for that) and
move the linear algebra related exceptions back to the linear package.
>
>>>>> [...]
>>>>
>>>> For example to have a single point where to put breakpoints for
>>>> debugging. I used that a lot as such exceptions mainly occur during
>>>> development phase.
>>>
>>> Thus, IIUC, this is a developer feature.
>>> I understand the convenience, but I wouldn't trade code consistency for it.
>>> I'd rather keep "NullArgumentException" (or a "MathNullPointerException" if
>>> we want it to extend the standard NPE) so that you can put your breakpoint
>>> in the constructor of that exception. Thus you have the convenience without
>>> the code asymmetry.
>>
>> OK for MathNullPointerException.
>
> With a "checkNotNull" function, it should not be necessary.
> But I didn't quite understood your reasoning concerning such a method and
> whether it's OK to implement or not (cf. below).
>
>>>
>>>>>
>>>>> Not only is this inconvenient, it is also dangerous: I was bitten more than
>>>>> once writing this:
>>>>> ---
>>>>> MathRuntimeException.createNullPointerException();
>>>>> ---
>>>>> which the compiler happily accepted.
>>>>
>>>> Of course, as it would have accepted:
>>>>
>>>> new NullPointerException();
>>>>
>>>> Unfortunately, we cannot put the throw inside the method. Trying to do
>>>> this prevent the optimizer to wor properly as it does not know in the
>>>> caller that the method would never return, and it brings lots of
>>>> warnings from code checking tools for the same reason.
>>>
>>> The method alternative looses much of its appeal because it doesn't perform
>>> the complete action ("throw" included), as I had initially assumed when
>>> wrongly leaving out the "throw" statement. I wouldn't have written
>>> new NullPointerException();
>>> as, to me, it would have _obviously_ looked wrong.
>>>
>>> I don't understand the problem with the "optimizer". There are several
>>> methods in CM that seem to perform similarly (i.e. they check something and
>>> throw an exception). An example is "checkOrder" in "MathUtils".
>>> In fact we could add a "checkNotNull" in "MathUtils":
>>> ---
>>> public checkNotNull(Object obj) {
>>> if (obj == null) {
>>> throw new NullPointerException();
>>> }
>>> }
>>> ---
>>> Wouldn't that satisfy all needs (ability to set a breakpoint and no factory
>>> methods)?
>>
>> No the check may succeed or not, so sometimes these methods do return.
>
> I'd say that they return most of the time, and that's fortunate... :-)
Sorry, I misunderstood what you were writing. I was still talking about
the changing the createXxxException (which only creates) into
throwXxxException (which creates and throws), not about
checkXxxCondition (which checks, creates and throws).
>
>> What I had in mind was something like:
>>
>> public static void throwNPE() {
>> throw new MySpecialNPE extends NullPointerException() {
>> // my special code here
>> };
>> }
>>
>> Such a method *never* returns but on the caller side, the compiler, and
>> the checking tools don't know it. Typically when you have code like:
>>
>> int var;
>> if (obj != null) {
>> var = obj.getVar();
>> } else {
>> throwNPE();
>> }
>>
>> // use var
>>
>> Then tools will complain that var may not be initialized, which is
>> false. However it is only because throwNPE never returns that it is false.
>
> Personally, I don't like this kind of syntax. The good programming rule is
> to assign at declaration. [Unfortunately, one is sometimes forced to write
> in this way and most often than not because of checked exceptions!]
>
>> Another way to put it that would probably work is:
>>
>> public static void checkNonNull(Object o) {
>> if (o == null) {
>> throw new MySpecialNPE extends NullPointerException() {
>> // my special code here
>> };
>> }
>> }
>
> How is this different from my version above?
It's the same, I simply get confused. So go for it, but including the
specialization and localization part that was in the former
createxxxException. So basically we add an if (obj == null) wrapper
aroud the 2.1 code.
best regards,
Luc
>
>> Then the caller would do:
>>
>> checkNonNull(obj);
>> int var = obj.getVar();
>
> That's exactly the intended usage!
>
>
> 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
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hello.
> [...]
> > So, what is the lesson from having this case? Is the whole method a duplicate
> > that should be replace by a call to something in "linear"? Or is it a proof
> > that some exceptions might be appropriately used outside of "linear"?
>
> I think the method should be moved to a new
> TruncatedCholeskyDecomposition or RectangularCholeskyDecomposition. I
> don't think we can add it has a new implementation or it should be added
> to the existing CholeskyDecomposition because the decomposition matrix
> shape is not triangular.
If you are going to create new classes in "linear" so that the current CM
code only throws "...MatrixException"s from inside that package, I'm not
going argue anymore about moving those exceptions to "linear". But, if this
situation arises again, then we will create a incompatibility by moving the
exception from "linear" to "exception"... That's why I think that it's safer
(from a stability point-of-view) to have them all in "exception".
> >>> [...]
> >>
> >> For example to have a single point where to put breakpoints for
> >> debugging. I used that a lot as such exceptions mainly occur during
> >> development phase.
> >
> > Thus, IIUC, this is a developer feature.
> > I understand the convenience, but I wouldn't trade code consistency for it.
> > I'd rather keep "NullArgumentException" (or a "MathNullPointerException" if
> > we want it to extend the standard NPE) so that you can put your breakpoint
> > in the constructor of that exception. Thus you have the convenience without
> > the code asymmetry.
>
> OK for MathNullPointerException.
With a "checkNotNull" function, it should not be necessary.
But I didn't quite understood your reasoning concerning such a method and
whether it's OK to implement or not (cf. below).
> >
> >>>
> >>> Not only is this inconvenient, it is also dangerous: I was bitten more than
> >>> once writing this:
> >>> ---
> >>> MathRuntimeException.createNullPointerException();
> >>> ---
> >>> which the compiler happily accepted.
> >>
> >> Of course, as it would have accepted:
> >>
> >> new NullPointerException();
> >>
> >> Unfortunately, we cannot put the throw inside the method. Trying to do
> >> this prevent the optimizer to wor properly as it does not know in the
> >> caller that the method would never return, and it brings lots of
> >> warnings from code checking tools for the same reason.
> >
> > The method alternative looses much of its appeal because it doesn't perform
> > the complete action ("throw" included), as I had initially assumed when
> > wrongly leaving out the "throw" statement. I wouldn't have written
> > new NullPointerException();
> > as, to me, it would have _obviously_ looked wrong.
> >
> > I don't understand the problem with the "optimizer". There are several
> > methods in CM that seem to perform similarly (i.e. they check something and
> > throw an exception). An example is "checkOrder" in "MathUtils".
> > In fact we could add a "checkNotNull" in "MathUtils":
> > ---
> > public checkNotNull(Object obj) {
> > if (obj == null) {
> > throw new NullPointerException();
> > }
> > }
> > ---
> > Wouldn't that satisfy all needs (ability to set a breakpoint and no factory
> > methods)?
>
> No the check may succeed or not, so sometimes these methods do return.
I'd say that they return most of the time, and that's fortunate... :-)
> What I had in mind was something like:
>
> public static void throwNPE() {
> throw new MySpecialNPE extends NullPointerException() {
> // my special code here
> };
> }
>
> Such a method *never* returns but on the caller side, the compiler, and
> the checking tools don't know it. Typically when you have code like:
>
> int var;
> if (obj != null) {
> var = obj.getVar();
> } else {
> throwNPE();
> }
>
> // use var
>
> Then tools will complain that var may not be initialized, which is
> false. However it is only because throwNPE never returns that it is false.
Personally, I don't like this kind of syntax. The good programming rule is
to assign at declaration. [Unfortunately, one is sometimes forced to write
in this way and most often than not because of checked exceptions!]
> Another way to put it that would probably work is:
>
> public static void checkNonNull(Object o) {
> if (o == null) {
> throw new MySpecialNPE extends NullPointerException() {
> // my special code here
> };
> }
> }
How is this different from my version above?
> Then the caller would do:
>
> checkNonNull(obj);
> int var = obj.getVar();
That's exactly the intended usage!
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Phil Steitz <ph...@gmail.com>.
On 3/4/11 10:47 AM, Luc Maisonobe wrote:
> Le 04/03/2011 13:55, Gilles Sadowski a écrit :
>>>>>>> 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).
>>> Right. I did not remember that (despite I wrote this code).
>> So, what is the lesson from having this case? Is the whole method a duplicate
>> that should be replace by a call to something in "linear"? Or is it a proof
>> that some exceptions might be appropriately used outside of "linear"?
I would say that regardless of the implementation choice, the
exception makes sense in this context (variance-covariance matrix
must be positive definite), so does mean it is useful outside of the
linear package, but it is a linear abstraction, so IMO should be
defined there.
Phil
> I think the method should be moved to a new
> TruncatedCholeskyDecomposition or RectangularCholeskyDecomposition. I
> don't think we can add it has a new implementation or it should be added
> to the existing CholeskyDecomposition because the decomposition matrix
> shape is not triangular.
>
>>>> [...]
>>>>
>>>> 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...]
>>> Fine.
>> I'll create a JIRA issue.
>>
>>>>>>>>> 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();
>>>> ---
>>>> ?
>>> For example to have a single point where to put breakpoints for
>>> debugging. I used that a lot as such exceptions mainly occur during
>>> development phase.
>> Thus, IIUC, this is a developer feature.
>> I understand the convenience, but I wouldn't trade code consistency for it.
>> I'd rather keep "NullArgumentException" (or a "MathNullPointerException" if
>> we want it to extend the standard NPE) so that you can put your breakpoint
>> in the constructor of that exception. Thus you have the convenience without
>> the code asymmetry.
> OK for MathNullPointerException.
>
>>>> Not only is this inconvenient, it is also dangerous: I was bitten more than
>>>> once writing this:
>>>> ---
>>>> MathRuntimeException.createNullPointerException();
>>>> ---
>>>> which the compiler happily accepted.
>>> Of course, as it would have accepted:
>>>
>>> new NullPointerException();
>>>
>>> Unfortunately, we cannot put the throw inside the method. Trying to do
>>> this prevent the optimizer to wor properly as it does not know in the
>>> caller that the method would never return, and it brings lots of
>>> warnings from code checking tools for the same reason.
>> The method alternative looses much of its appeal because it doesn't perform
>> the complete action ("throw" included), as I had initially assumed when
>> wrongly leaving out the "throw" statement. I wouldn't have written
>> new NullPointerException();
>> as, to me, it would have _obviously_ looked wrong.
>>
>> I don't understand the problem with the "optimizer". There are several
>> methods in CM that seem to perform similarly (i.e. they check something and
>> throw an exception). An example is "checkOrder" in "MathUtils".
>> In fact we could add a "checkNotNull" in "MathUtils":
>> ---
>> public checkNotNull(Object obj) {
>> if (obj == null) {
>> throw new NullPointerException();
>> }
>> }
>> ---
>> Wouldn't that satisfy all needs (ability to set a breakpoint and no factory
>> methods)?
> No the check may succeed or not, so sometimes these methods do return.
>
> What I had in mind was something like:
>
> public static void throwNPE() {
> throw new MySpecialNPE extends NullPointerException() {
> // my special code here
> };
> }
>
> Such a method *never* returns but on the caller side, the compiler, and
> the checking tools don't know it. Typically when you have code like:
>
> int var;
> if (obj != null) {
> var = obj.getVar();
> } else {
> throwNPE();
> }
>
> // use var
>
> Then tools will complain that var may not be initialized, which is
> false. However it is only because throwNPE never returns that it is false.
>
> Another way to put it that would probably work is:
>
> public static void checkNonNull(Object o) {
> if (o == null) {
> throw new MySpecialNPE extends NullPointerException() {
> // my special code here
> };
> }
> }
>
> Then the caller would do:
>
> checkNonNull(obj);
> int var = obj.getVar();
>
>
> Luc
>
>>
>> Regards,
>> Gilles
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Ole Ersoy <ol...@gmail.com>.
Hi,
Just want to throw another idea into the mix. How about stripping all the validation argument validation lines and putting in separate validator classes. Clients would call the validator on the arguments for the class before passing the arguments to the class's method. It makes the core classes more focused and efficient and enables clients to wrap method invocations with aspects that apply the validation routine.
Cheers,
- Ole
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Luc Maisonobe <Lu...@free.fr>.
Le 04/03/2011 13:55, Gilles Sadowski a écrit :
>>>>>> 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).
>>
>> Right. I did not remember that (despite I wrote this code).
>
> So, what is the lesson from having this case? Is the whole method a duplicate
> that should be replace by a call to something in "linear"? Or is it a proof
> that some exceptions might be appropriately used outside of "linear"?
I think the method should be moved to a new
TruncatedCholeskyDecomposition or RectangularCholeskyDecomposition. I
don't think we can add it has a new implementation or it should be added
to the existing CholeskyDecomposition because the decomposition matrix
shape is not triangular.
>
>>> [...]
>>>
>>> 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...]
>>
>> Fine.
>
> I'll create a JIRA issue.
>
>>>>>
>>>>>>>
>>>>>>>> 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();
>>> ---
>>> ?
>>
>> For example to have a single point where to put breakpoints for
>> debugging. I used that a lot as such exceptions mainly occur during
>> development phase.
>
> Thus, IIUC, this is a developer feature.
> I understand the convenience, but I wouldn't trade code consistency for it.
> I'd rather keep "NullArgumentException" (or a "MathNullPointerException" if
> we want it to extend the standard NPE) so that you can put your breakpoint
> in the constructor of that exception. Thus you have the convenience without
> the code asymmetry.
OK for MathNullPointerException.
>
>>>
>>> Not only is this inconvenient, it is also dangerous: I was bitten more than
>>> once writing this:
>>> ---
>>> MathRuntimeException.createNullPointerException();
>>> ---
>>> which the compiler happily accepted.
>>
>> Of course, as it would have accepted:
>>
>> new NullPointerException();
>>
>> Unfortunately, we cannot put the throw inside the method. Trying to do
>> this prevent the optimizer to wor properly as it does not know in the
>> caller that the method would never return, and it brings lots of
>> warnings from code checking tools for the same reason.
>
> The method alternative looses much of its appeal because it doesn't perform
> the complete action ("throw" included), as I had initially assumed when
> wrongly leaving out the "throw" statement. I wouldn't have written
> new NullPointerException();
> as, to me, it would have _obviously_ looked wrong.
>
> I don't understand the problem with the "optimizer". There are several
> methods in CM that seem to perform similarly (i.e. they check something and
> throw an exception). An example is "checkOrder" in "MathUtils".
> In fact we could add a "checkNotNull" in "MathUtils":
> ---
> public checkNotNull(Object obj) {
> if (obj == null) {
> throw new NullPointerException();
> }
> }
> ---
> Wouldn't that satisfy all needs (ability to set a breakpoint and no factory
> methods)?
No the check may succeed or not, so sometimes these methods do return.
What I had in mind was something like:
public static void throwNPE() {
throw new MySpecialNPE extends NullPointerException() {
// my special code here
};
}
Such a method *never* returns but on the caller side, the compiler, and
the checking tools don't know it. Typically when you have code like:
int var;
if (obj != null) {
var = obj.getVar();
} else {
throwNPE();
}
// use var
Then tools will complain that var may not be initialized, which is
false. However it is only because throwNPE never returns that it is false.
Another way to put it that would probably work is:
public static void checkNonNull(Object o) {
if (o == null) {
throw new MySpecialNPE extends NullPointerException() {
// my special code here
};
}
}
Then the caller would do:
checkNonNull(obj);
int var = obj.getVar();
Luc
>
>
> 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
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> >>>> 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).
>
> Right. I did not remember that (despite I wrote this code).
So, what is the lesson from having this case? Is the whole method a duplicate
that should be replace by a call to something in "linear"? Or is it a proof
that some exceptions might be appropriately used outside of "linear"?
> > [...]
> >
> > 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...]
>
> Fine.
I'll create a JIRA issue.
> >>>
> >>>>>
> >>>>>> 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();
> > ---
> > ?
>
> For example to have a single point where to put breakpoints for
> debugging. I used that a lot as such exceptions mainly occur during
> development phase.
Thus, IIUC, this is a developer feature.
I understand the convenience, but I wouldn't trade code consistency for it.
I'd rather keep "NullArgumentException" (or a "MathNullPointerException" if
we want it to extend the standard NPE) so that you can put your breakpoint
in the constructor of that exception. Thus you have the convenience without
the code asymmetry.
> >
> > Not only is this inconvenient, it is also dangerous: I was bitten more than
> > once writing this:
> > ---
> > MathRuntimeException.createNullPointerException();
> > ---
> > which the compiler happily accepted.
>
> Of course, as it would have accepted:
>
> new NullPointerException();
>
> Unfortunately, we cannot put the throw inside the method. Trying to do
> this prevent the optimizer to wor properly as it does not know in the
> caller that the method would never return, and it brings lots of
> warnings from code checking tools for the same reason.
The method alternative looses much of its appeal because it doesn't perform
the complete action ("throw" included), as I had initially assumed when
wrongly leaving out the "throw" statement. I wouldn't have written
new NullPointerException();
as, to me, it would have _obviously_ looked wrong.
I don't understand the problem with the "optimizer". There are several
methods in CM that seem to perform similarly (i.e. they check something and
throw an exception). An example is "checkOrder" in "MathUtils".
In fact we could add a "checkNotNull" in "MathUtils":
---
public checkNotNull(Object obj) {
if (obj == null) {
throw new NullPointerException();
}
}
---
Wouldn't that satisfy all needs (ability to set a breakpoint and no factory
methods)?
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Luc Maisonobe <Lu...@free.fr>.
Le 03/03/2011 23:51, Gilles Sadowski a écrit :
> 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).
Right. I did not remember that (despite I wrote this code).
>
>>> 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...]
Fine.
>
>>>
>>>>>
>>>>>> 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();
> ---
> ?
For example to have a single point where to put breakpoints for
debugging. I used that a lot as such exceptions mainly occur during
development phase.
>
> Not only is this inconvenient, it is also dangerous: I was bitten more than
> once writing this:
> ---
> MathRuntimeException.createNullPointerException();
> ---
> which the compiler happily accepted.
Of course, as it would have accepted:
new NullPointerException();
Unfortunately, we cannot put the throw inside the method. Trying to do
this prevent the optimizer to wor properly as it does not know in the
caller that the method would never return, and it brings lots of
warnings from code checking tools for the same reason.
Luc
>
>>>
>>>>>> 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
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Phil Steitz <ph...@gmail.com>.
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
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
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.
> >
> >>>> 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
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Luc Maisonobe <Lu...@free.fr>.
Le 03/03/2011 16:34, Gilles Sadowski a écrit :
> Hi Luc,
>
> Thanks for repeating that you agree with most points, as was the case with
> all the changes I had been implementing in "trunk".
> The discussion went awry when, because of a single point of disagreement
> (about whether a user-only "FunctionEvaluationException" should be defined
> in CM), all the work on the exception revamping of these past 6 months has
> been threatened of nullification based on matters of taste.
>
> Below, I've heavily cut the quoted parts of the thread because it has become
> fairly difficult to distinguish the new information.
>
>>>> 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.
>>>
>>> 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).
>
> I was asking because you said: "[...] public Request For Proposal [...]".
>
>>>
>>>> 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.
>
> 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.
>
> 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.
>
>>>
>>>> 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 ?
>
>>>> 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.
Luc
>
>
> Best,
> 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
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi Luc,
Thanks for repeating that you agree with most points, as was the case with
all the changes I had been implementing in "trunk".
The discussion went awry when, because of a single point of disagreement
(about whether a user-only "FunctionEvaluationException" should be defined
in CM), all the work on the exception revamping of these past 6 months has
been threatened of nullification based on matters of taste.
Below, I've heavily cut the quoted parts of the thread because it has become
fairly difficult to distinguish the new information.
> >> 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.
> >
> > 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).
I was asking because you said: "[...] public Request For Proposal [...]".
> >
> >> 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.
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".
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.
> >
> >> 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.
> >
> >> 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.
> >> 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.
Best,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Michael Giannakopoulos <mi...@gmail.com>.
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
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Luc Maisonobe <Lu...@free.fr>.
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
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Phil Steitz <ph...@gmail.com>.
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: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Luc Maisonobe <Lu...@free.fr>.
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: 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
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Stephen Colebourne <sc...@joda.org>.
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 <pb...@apache.org> 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 <
> adrian.crum@sandglass-software.com> 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: 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
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Paul Benedict <pb...@apache.org>.
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 <
adrian.crum@sandglass-software.com> 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: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Adrian Crum <ad...@sandglass-software.com>.
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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Stephen Colebourne <sc...@joda.org>.
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 <pb...@apache.org> 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 <bi...@verizon.net> wrote:
>
>>
>>
>> -----Original Message----- From: Gilles Sadowski
>> Sent: Tuesday, March 01, 2011 3:12 PM
>> To: dev@commons.apache.org
>> 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: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Paul Benedict <pb...@apache.org>.
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 <bi...@verizon.net> wrote:
>
>
> -----Original Message----- From: Gilles Sadowski
> Sent: Tuesday, March 01, 2011 3:12 PM
> To: dev@commons.apache.org
> 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: 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
>
>
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> > [...]
>
> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Bill Barker <bi...@verizon.net>.
-----Original Message-----
From: Gilles Sadowski
Sent: Tuesday, March 01, 2011 3:12 PM
To: dev@commons.apache.org
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: 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
Re: [Math - 403] Never propagate a "NullPointerException"
resulting from bad usage of the API
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math - 403] Never propagate a "NullPointerException" resulting
from bad usage of the API
Posted by Paul Benedict <pb...@apache.org>.
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 <miccagiann@gmail.com
> 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
>