You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by lu...@apache.org on 2012/09/05 20:30:08 UTC

svn commit: r1381284 - in /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex: Complex.java ComplexFormat.java ComplexUtils.java

Author: luc
Date: Wed Sep  5 18:30:08 2012
New Revision: 1381284

URL: http://svn.apache.org/viewvc?rev=1381284&view=rev
Log:
Added throw declarations for package complex.

Modified:
    commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
    commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java
    commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java

Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284&r1=1381283&r2=1381284&view=diff
==============================================================================
--- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java (original)
+++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java Wed Sep  5 18:30:08 2012
@@ -22,6 +22,7 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.commons.math3.FieldElement;
+import org.apache.commons.math3.exception.MathInternalError;
 import org.apache.commons.math3.exception.NullArgumentException;
 import org.apache.commons.math3.exception.NotPositiveException;
 import org.apache.commons.math3.exception.util.LocalizedFormats;
@@ -566,12 +567,17 @@ public class Complex implements FieldEle
      * @since 1.2
      */
     public Complex acos() {
-        if (isNaN) {
-            return NaN;
-        }
+        try {
+            if (isNaN) {
+                return NaN;
+            }
 
-        return this.add(this.sqrt1z().multiply(I)).log()
-            .multiply(I.negate());
+            return this.add(this.sqrt1z().multiply(I)).log()
+                    .multiply(I.negate());
+        } catch (NullArgumentException e) {
+            // this should never happen as intermediat results are not null
+            throw new MathInternalError(e);
+        }
     }
 
     /**
@@ -591,12 +597,17 @@ public class Complex implements FieldEle
      * @since 1.2
      */
     public Complex asin() {
-        if (isNaN) {
-            return NaN;
-        }
+        try {
+            if (isNaN) {
+                return NaN;
+            }
 
-        return sqrt1z().add(this.multiply(I)).log()
-            .multiply(I.negate());
+            return sqrt1z().add(this.multiply(I)).log()
+                    .multiply(I.negate());
+        } catch (NullArgumentException e) {
+            // this should never happen as intermediat results are not null
+            throw new MathInternalError(e);
+        }
     }
 
     /**
@@ -616,12 +627,17 @@ public class Complex implements FieldEle
      * @since 1.2
      */
     public Complex atan() {
-        if (isNaN) {
-            return NaN;
-        }
+        try {
+            if (isNaN) {
+                return NaN;
+            }
 
-        return this.add(I).divide(I.subtract(this)).log()
-            .multiply(I.divide(createComplex(2.0, 0.0)));
+            return this.add(I).divide(I.subtract(this)).log()
+                    .multiply(I.divide(createComplex(2.0, 0.0)));
+        } catch (NullArgumentException e) {
+            // this should never happen as intermediat results are not null
+            throw new MathInternalError(e);
+        }
     }
 
     /**
@@ -979,7 +995,12 @@ public class Complex implements FieldEle
      * @since 1.2
      */
     public Complex sqrt1z() {
-        return createComplex(1.0, 0.0).subtract(this.multiply(this)).sqrt();
+        try {
+            return createComplex(1.0, 0.0).subtract(this.multiply(this)).sqrt();
+        } catch (NullArgumentException e) {
+            // this should never happen as intermediat results are not null
+            throw new MathInternalError(e);
+        }
     }
 
     /**
@@ -1128,7 +1149,7 @@ public class Complex implements FieldEle
      * @throws NotPositiveException if {@code n <= 0}.
      * @since 2.0
      */
-    public List<Complex> nthRoot(int n) {
+    public List<Complex> nthRoot(int n) throws NotPositiveException {
 
         if (n <= 0) {
             throw new NotPositiveException(LocalizedFormats.CANNOT_COMPUTE_NTH_ROOT_FOR_NEGATIVE_N,

Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java?rev=1381284&r1=1381283&r2=1381284&view=diff
==============================================================================
--- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java (original)
+++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java Wed Sep  5 18:30:08 2012
@@ -22,13 +22,13 @@ import java.text.NumberFormat;
 import java.text.ParsePosition;
 import java.util.Locale;
 
-import org.apache.commons.math3.util.CompositeFormat;
-import org.apache.commons.math3.exception.util.LocalizedFormats;
-import org.apache.commons.math3.exception.MathParseException;
 import org.apache.commons.math3.exception.MathIllegalArgumentException;
 import org.apache.commons.math3.exception.MathInternalError;
-import org.apache.commons.math3.exception.NullArgumentException;
+import org.apache.commons.math3.exception.MathParseException;
 import org.apache.commons.math3.exception.NoDataException;
+import org.apache.commons.math3.exception.NullArgumentException;
+import org.apache.commons.math3.exception.util.LocalizedFormats;
+import org.apache.commons.math3.util.CompositeFormat;
 
 /**
  * Formats a Complex number in cartesian format "Re(c) + Im(c)i".  'i' can
@@ -53,16 +53,24 @@ public class ComplexFormat {
      * default number format for both real and imaginary parts.
      */
     public ComplexFormat() {
-        this(DEFAULT_IMAGINARY_CHARACTER, CompositeFormat.getDefaultNumberFormat());
+        this.imaginaryCharacter = DEFAULT_IMAGINARY_CHARACTER;
+        this.imaginaryFormat = CompositeFormat.getDefaultNumberFormat();
+        this.realFormat = imaginaryFormat;
     }
 
     /**
      * Create an instance with a custom number format for both real and
      * imaginary parts.
      * @param format the custom format for both real and imaginary parts.
+     * @throws NullArgumentException if {@code realFormat} is {@code null}.
      */
-    public ComplexFormat(NumberFormat format) {
-        this(DEFAULT_IMAGINARY_CHARACTER, format);
+    public ComplexFormat(NumberFormat format) throws NullArgumentException {
+        if (format == null) {
+            throw new NullArgumentException(LocalizedFormats.IMAGINARY_FORMAT);
+        }
+        this.imaginaryCharacter = DEFAULT_IMAGINARY_CHARACTER;
+        this.imaginaryFormat = format;
+        this.realFormat = format;
     }
 
     /**
@@ -70,17 +78,34 @@ public class ComplexFormat {
      * custom number format for the imaginary part.
      * @param realFormat the custom format for the real part.
      * @param imaginaryFormat the custom format for the imaginary part.
-     */
-    public ComplexFormat(NumberFormat realFormat, NumberFormat imaginaryFormat) {
-        this(DEFAULT_IMAGINARY_CHARACTER, realFormat, imaginaryFormat);
+     * @throws NullArgumentException if {@code imaginaryFormat} is {@code null}.
+     * @throws NullArgumentException if {@code realFormat} is {@code null}.
+      */
+    public ComplexFormat(NumberFormat realFormat, NumberFormat imaginaryFormat)
+        throws NullArgumentException {
+        if (imaginaryFormat == null) {
+            throw new NullArgumentException(LocalizedFormats.IMAGINARY_FORMAT);
+        }
+        if (realFormat == null) {
+            throw new NullArgumentException(LocalizedFormats.REAL_FORMAT);
+        }
+
+        this.imaginaryCharacter = DEFAULT_IMAGINARY_CHARACTER;
+        this.imaginaryFormat = imaginaryFormat;
+        this.realFormat = realFormat;
     }
 
     /**
      * Create an instance with a custom imaginary character, and the default
      * number format for both real and imaginary parts.
      * @param imaginaryCharacter The custom imaginary character.
+     * @throws NullArgumentException if {@code imaginaryCharacter} is
+     * {@code null}.
+     * @throws NoDataException if {@code imaginaryCharacter} is an
+     * empty string.
      */
-    public ComplexFormat(String imaginaryCharacter) {
+    public ComplexFormat(String imaginaryCharacter)
+        throws NullArgumentException, NoDataException {
         this(imaginaryCharacter, CompositeFormat.getDefaultNumberFormat());
     }
 
@@ -89,8 +114,14 @@ public class ComplexFormat {
      * format for both real and imaginary parts.
      * @param imaginaryCharacter The custom imaginary character.
      * @param format the custom format for both real and imaginary parts.
+     * @throws NullArgumentException if {@code imaginaryCharacter} is
+     * {@code null}.
+     * @throws NoDataException if {@code imaginaryCharacter} is an
+     * empty string.
+     * @throws NullArgumentException if {@code format} is {@code null}.
      */
-    public ComplexFormat(String imaginaryCharacter, NumberFormat format) {
+    public ComplexFormat(String imaginaryCharacter, NumberFormat format)
+        throws NullArgumentException, NoDataException {
         this(imaginaryCharacter, format, format);
     }
 
@@ -111,7 +142,8 @@ public class ComplexFormat {
      */
     public ComplexFormat(String imaginaryCharacter,
                          NumberFormat realFormat,
-                         NumberFormat imaginaryFormat) {
+                         NumberFormat imaginaryFormat)
+        throws NullArgumentException, NoDataException {
         if (imaginaryCharacter == null) {
             throw new NullArgumentException();
         }
@@ -235,10 +267,11 @@ public class ComplexFormat {
      *            offsets of the alignment field
      * @return the value passed in as toAppendTo.
      * @see java.text.Format#format(java.lang.Object, java.lang.StringBuffer, java.text.FieldPosition)
-     * @throws IllegalArgumentException is {@code obj} is not a valid type.
+     * @throws MathIllegalArgumentException is {@code obj} is not a valid type.
      */
     public StringBuffer format(Object obj, StringBuffer toAppendTo,
-                               FieldPosition pos) {
+                               FieldPosition pos)
+        throws MathIllegalArgumentException {
 
         StringBuffer ret = null;
 
@@ -285,8 +318,13 @@ public class ComplexFormat {
      * @return the complex format specific to the given locale.
      */
     public static ComplexFormat getInstance(Locale locale) {
-        NumberFormat f = CompositeFormat.getDefaultNumberFormat(locale);
-        return new ComplexFormat(f);
+        try {
+            NumberFormat f = CompositeFormat.getDefaultNumberFormat(locale);
+            return new ComplexFormat(f);
+        } catch (NullArgumentException nae) {
+            // this should never happen
+            throw new MathInternalError(nae);
+        }
     }
 
     /**
@@ -294,9 +332,13 @@ public class ComplexFormat {
      * @param locale the specific locale used by the format.
      * @param imaginaryCharacter Imaginary character.
      * @return the complex format specific to the given locale.
+     * @throws NullArgumentException if {@code imaginaryCharacter} is
+     * {@code null}.
+     * @throws NoDataException if {@code imaginaryCharacter} is an
+     * empty string.
      */
-    public static ComplexFormat getInstance(String imaginaryCharacter,
-                                            Locale locale) {
+    public static ComplexFormat getInstance(String imaginaryCharacter, Locale locale)
+        throws NullArgumentException, NoDataException {
         NumberFormat f = CompositeFormat.getDefaultNumberFormat(locale);
         return new ComplexFormat(imaginaryCharacter, f);
     }
@@ -317,7 +359,7 @@ public class ComplexFormat {
      * @throws MathParseException if the beginning of the specified string
      * cannot be parsed.
      */
-    public Complex parse(String source) {
+    public Complex parse(String source) throws MathParseException {
         ParsePosition parsePosition = new ParsePosition(0);
         Complex result = parse(source, parsePosition);
         if (parsePosition.getIndex() == 0) {

Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java?rev=1381284&r1=1381283&r2=1381284&view=diff
==============================================================================
--- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java (original)
+++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java Wed Sep  5 18:30:08 2012
@@ -59,7 +59,7 @@ public class ComplexUtils {
      * @throws MathIllegalArgumentException if {@code r} is negative.
      * @since 1.1
      */
-    public static Complex polar2Complex(double r, double theta) {
+    public static Complex polar2Complex(double r, double theta) throws MathIllegalArgumentException {
         if (r < 0) {
             throw new MathIllegalArgumentException(
                   LocalizedFormats.NEGATIVE_COMPLEX_MODULE, r);



Re: [Math] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hi Gilles,

2012/9/17 Luc Maisonobe <Lu...@free.fr>:
> Le 17/09/2012 11:50, Gilles Sadowski a écrit :
>> On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote:
>>> OK, I give up.  Lets do option 2.  Just warn users in the User Guide
>>> somewhere that our APIs are in general not null-safe and unless the
>>> javadoc explicitly allows nulls, they can expect NPE when passing nulls.
>>
>> Thanks, Phil; we are making progress, and I hope that you'll be convinced of
>> that at some point.
>>
>> Another decision still needs to be made.
>>
>> I think that everybody now agrees that wherever an argument will be directly
>> used (i.e. "dereferenced") inside the body of the method[1], it is safe to
>> not check for null (since the JVM will throw an NPE).
>>
>> But, whenever an argument is passed for _later_ use (e.g. stored by a
>> constructor or passed to another method), we also all expressed that it is
>> better to fail early if the object is supposed to be non-null. In those
>> cases, checks are not mandatory (since the JVM will throw NPE at some
>> point), but we must agree on how optional checks are to be implemented.
>> 1. The current state of affairs was to throw "NullArgumentException"; in 4.0
>>    this exception will become a subclass of NPE and we can continue to use
>>    it in the same way as now (i.e. possibly providing additional localizable
>>    error messages).
>> 2. The alternative is to directly throw a standard NPE, but without any
>>    message, since it wouldn't be localizable with the support of CM's
>>    "context".
>>
>> So, which of those alternatives do people want to settle on?
>
> I would prefer 1. If we fail early, do it as precisely as possible.
>
> Luc
>
>>
>>
>> Regards,
>> Gilles
>>
>> [1] As opposed to being passed on to another method.
>>
I would also be in favor of #1, so that any exception explicitely
thrown by CM is one of our own exceptions.
Sébastien


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 17/09/2012 11:50, Gilles Sadowski a écrit :
> On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote:
>> OK, I give up.  Lets do option 2.  Just warn users in the User Guide
>> somewhere that our APIs are in general not null-safe and unless the
>> javadoc explicitly allows nulls, they can expect NPE when passing nulls.
> 
> Thanks, Phil; we are making progress, and I hope that you'll be convinced of
> that at some point.
> 
> Another decision still needs to be made.
> 
> I think that everybody now agrees that wherever an argument will be directly
> used (i.e. "dereferenced") inside the body of the method[1], it is safe to
> not check for null (since the JVM will throw an NPE).
> 
> But, whenever an argument is passed for _later_ use (e.g. stored by a
> constructor or passed to another method), we also all expressed that it is
> better to fail early if the object is supposed to be non-null. In those
> cases, checks are not mandatory (since the JVM will throw NPE at some
> point), but we must agree on how optional checks are to be implemented.
> 1. The current state of affairs was to throw "NullArgumentException"; in 4.0
>    this exception will become a subclass of NPE and we can continue to use
>    it in the same way as now (i.e. possibly providing additional localizable
>    error messages).
> 2. The alternative is to directly throw a standard NPE, but without any
>    message, since it wouldn't be localizable with the support of CM's
>    "context".
> 
> So, which of those alternatives do people want to settle on?

I would prefer 1. If we fail early, do it as precisely as possible.

Luc

> 
> 
> Regards,
> Gilles
> 
> [1] As opposed to being passed on to another method.
> 
>> Phil
>>
>> On 9/14/12 8:46 AM, Gilles Sadowski wrote:
>>> On Fri, Sep 14, 2012 at 07:44:08AM -0700, Phil Steitz wrote:
>>>> On 9/14/12 4:28 AM, Gilles Sadowski wrote:
>>>>> On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote:
>>>>>> Le 14/09/2012 08:46, Sébastien Brisard a écrit :
>>>>>>> Hi Phil
>>>>>>>
>>>>>>>>> Back to square one, with 3 fully consistent alternatives:
>>>>>>>>>  1. CM to always check for null? Then "NullArgumentException" inheriting from
>>>>>>>>>     "MathIllegalArgumentException" is fine because we promise to the user that
>>>>>>>>>     no NPE will ever propagate through the CM layer. [Breaking that promise
>>>>>>>>>     is a CM bug.]
>>>>>>>>>  2. CM to never check for null? Then we delete class "NullArgumentException".
>>>>>>>>>     Users are warned by the general policy: "Do not pass null unless it is
>>>>>>>>>     explicitly documented to be allowed." A bug will lead to the JVM raising
>>>>>>>>>     a NPE.
>>>>>>>>>  3. CM to sometimes check for null? Then "NullArgumentException" should
>>>>>>>>>     inherit from "NullPointerException" because the user will sometimes see
>>>>>>>>>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
>>>>>>>>>     not check) and both should thus belong to the same hierarchy (from the
>>>>>>>>>     user's point-of-view, there is no reason to handle them in different
>>>>>>>>>     ways since it's the exact same bug).
>>>>>>>>>     For the user, the consequence will be similar to alternative 2, with
>>>>>>>>>     sometimes more information about the failure and sometimes (marginally)
>>>>>>>>>     earlier detection.
>>>>>>>> As stated above, my preference is
>>>>>>>>
>>>>>>>> 4.  CM APIs *may* disallow nulls explicitly.  When that is done, the
>>>>>>>> implementations *should* check parameters (as they *should* check
>>>>>>>> all other stated preconditions) and when preconditions are violated,
>>>>>>>> a MathIllegalArgumentException is thrown.  I don't care whether or
>>>>>>>> not we keep NAE.  If we drop it, we should make sure whatever
>>>>>>>> exception messages we used to use to indicate illegal null arguments
>>>>>>>> are still there.
>>>>>>>>
>>>>>>>> Phil
>>>>>>>>
>>>>>>> I like this option better than 3. So, I'll change my "vote" to option
>>>>>>> #2, and possibly option #4 as we will never agree on option #2.
>>>>> Why is it better to throw MathIllegalArgumentException rather than NPE?
>>>> Because, as I have repeated numerous times, that is the appropriate
>>>> exception to throw when API preconditions are violated.  That is why
>>>> IAE exists.  The class javadoc comment for IAE is nice and succinct
>>>> (we could learn from it ;)
>>> IllegalArgumentException:
>>> ---
>>> Thrown to indicate that a method has been passed an illegal or inappropriate
>>> argument. 
>>> ---
>>>
>>> Did you read the class Javadoc for NPE?
>>> Here is the last sentence pasted again:
>>> ---
>>> Applications should throw instances of this class to indicate other illegal
>>> uses of the null object. 
>>> ---
>>>
>>>>   You could ask the same question about
>>>> ArrayIndexOutOfBounds.  Why not throw that instead of IAE when bad
>>>> indices are provided?
>>> Does CM check _array_ index? No.
>>> Surprised? An "ArrayRealVector" is _not_ an array, it is the representation
>>> of the vector concept. That it is a backed by a Java array is an implemtation
>>> detail that should not surface to the API. Hence the choice to not use the
>>> low-level ArrayIndexOutOfBoundsException can be given a rationale.
>>> CM checks that you access valid entries of the high-level "mathematical"
>>> object. And we do that for _all_ such accesses and never (or it is a CM bug)
>>> let propagate an exception that reveals an underlying Java array.
>>> [This was one of my proposals too (alternative 1). However it imposes a
>>> unnecessary burden of duplicating (CM and JVM) every null check just for the
>>> sake of hiding NPE.]
>>>
>>> As indicated in a previous post, "null" is not a "RealVector", it is not an
>>> "Object", it is not a reference, it is just the value of an unitialized
>>> pointer.
>>>
>>>>   The difference is that instead of letting
>>>> the precondition failure go undetected and the runtime exception
>>>> propagate, APIs with explicit preconditions and parameter checking
>>>> in place raise the exception at method activation time.
>>> Could you please stop mentioning this, since my proposal (alternative 3)
>>> allows for early detection?
>>>
>>> The difference is that you want to use MathIAE, where everyone else uses NPE.
>>>
>>>>  In that
>>>> context, what is appropriate is IAE.
>>> You are stuck with a "name", as if everything that is "illegal" must be
>>> signalled with an exception that contains the string "Illegal" in its name.
>>>
>>> If that is so, I propose to rename "NullArgumentException" to
>>> "IllegalNullArgumentException".
>>>
>>>
>>> Gilles
>>>
>>>> Phil
>>>>>> My preferences are 4 or 3 with same preference level and 2 as a fallback
>>>>>> choice. For each option, I find the arguments are good, it's only a
>>>>>> matter or preference.
>>>>> Did everyone consider that Phil's alternative implies a behaviour that
>>>>> departs from the standard practice, a.o. 
>>>>>  * JDK (cf. quote in a previous post),
>>>>>  * other libraries, e.g. Guava (cf. quote in a previous post), and
>>>>>  * Java experts, e.g. Joshua Bloch[1]
>>>>> of throwing NPE when they encounter "null"?
>>>>> Doesn't anyone care about having a _reason_ for CM to behave differently?
>>>>>
>>>>> Don't you care that users will see different exceptions when the same
>>>>> problem is detected?
>>>>>
>>>>> Do you really like a policy that mandates different behaviours when "null"
>>>>> is disallowed implicitly vs explicitly (throwing NPE vs throwing
>>>>> MathRuntimeException)?  [Pardon me, but IMHO this is nonsense.]
>>>>>
>>>>> After piling up arguments (_none_ of which have been addressed), I'm sorry
>>>>> to read that it would only be a "matter of preference"! [I should start a
>>>>> career as a writer if I'm able to express "preference" in so many words...]
>>>>> Actually, it's a matter of consistency ("same problem, same exception").
>>>>> If nobody cares about improving CM's consistency, then indeed most of the
>>>>> discussions in which I take part are pointless.
>>>>>
>>>>> What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows
>>>>> CM to fail as early as possible (which I thought was Phil's main point, as
>>>>> it was his only one, apart from "preference"), while being consistent ("same
>>>>> problem, same exception"), internally, externally, implicitly and
>>>>> explicitly. Alternative 4 is inconsistent ("same problem, different
>>>>> exceptions"); it's a truth, not a matter of taste.]
>>>>>
>>>>> Rather then being willingly stuck in a deadlock because only the weakest
>>>>> argument ("preference") is taken into account, wouldn't it be more
>>>>> reasonable to count all the arguments?
>>>>>
>>>>>
>>>>> Gilles
>>>>>
>>>>> [1] "Effective Java" (second edition). Excerpt from "Item 60":
>>>>>     If a caller passes null in some parameter for which null values are
>>>>>     prohibited, convention dictates that NullPointerException be thrown
>>>>>     rather than IllegalArgumentException.
>>>>>
>>>>>> Luc
>>>>>>
>>>>>>> Best regards,
>>>>>>> Sébastien
>>>>>>>>> Your alternative (sometimes check, sometimes not) is not fully consistent:
>>>>>>>>>  * for the user: "In case of null reference, will I get a MathRuntimeException
>>>>>>>>>    or a NPE?"
>>>>>>>>>  * for the CM developer: "In which cases do I need to check for null?"
>>>>>>>>>
>>>>>>>>> Of course, I would reconsider if you could provide an actual example that
>>>>>>>>> would fail with all three alternatives which I suggested. If not, then
>>>>>>>>> it seems obvious that your alternative presents two defects that don't exist
>>>>>>>>> in any of those three.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Gilles
>>>>>>>>>
>>>>>>>>>>>> [...] what is different about null arguments at the point of
>>>>>>>>>>>> method activation in an API that explicitly disallows them.
>>>>>>>>>>> The difference is that there is no need to tell the user what the problem
>>>>>>>>>>> is because the raised exception says it all: "You tried to use a null
>>>>>>>>>>> reference." [As said above, the only issue is _when_ the exception is
>>>>>>>>>>> triggered.]
>>>>>>>>>>>
>>>>>>>>>>> The policy mandates what is globally valid, e.g.:
>>>>>>>>>>>   "If not specified otherwise, "null" is not allowed as an argument."
>>>>>>>>>>> Then, a user cannot complain about a NPE being propagated when he passed an
>>>>>>>>>>> invalid (null) argument.
>>>>>>>>>>>
>>>>>>>>>>> Finally, the case for "null" is also slightly peculiar (given the above)
>>>>>>>>>>> that it should not simply be bundled with the rationale "Fail early": Indeed
>>>>>>>>>>> "null" references always fail early (i.e. as soon as they are used).
>>>>>>>>>>> Deferring the check until it is done by the JVM will never entails the code
>>>>>>>>>>> to produce a wrong result _because_ of that null reference (it will just
>>>>>>>>>>> fail in the predictable way: NPE).[1]
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Gilles
>>>>>>>>>>>
>>>>>>>>>>> [1] Unlike in C, where an unitialized pointer would not necessarily crash
>>>>>>>>>>>     the program, but _could_ make it behave erratically (including produce
>>>>>>>>>>>     wrong results in a stealth way).
>>>>>>>>>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/19/12 12:12 AM, Sébastien Brisard wrote:
> Hi,
>
> 2012/9/18 Phil Steitz <ph...@gmail.com>:
>> On 9/18/12 12:02 PM, Sébastien Brisard wrote:
>>> Hello,
>>>
>>> 2012/9/17 Gilles Sadowski <gi...@harfang.homelinux.org>:
>>>> On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote:
>>>>> OK, I give up.  Lets do option 2.  Just warn users in the User Guide
>>>>> somewhere that our APIs are in general not null-safe and unless the
>>>>> javadoc explicitly allows nulls, they can expect NPE when passing nulls.
>>>> Thanks, Phil; we are making progress, and I hope that you'll be convinced of
>>>> that at some point.
>>>>
>>>> Another decision still needs to be made.
>>>>
>>>> I think that everybody now agrees that wherever an argument will be directly
>>>> used (i.e. "dereferenced") inside the body of the method[1], it is safe to
>>>> not check for null (since the JVM will throw an NPE).
>>>>
>>>> But, whenever an argument is passed for _later_ use (e.g. stored by a
>>>> constructor or passed to another method), we also all expressed that it is
>>>> better to fail early if the object is supposed to be non-null. In those
>>>> cases, checks are not mandatory (since the JVM will throw NPE at some
>>>> point), but we must agree on how optional checks are to be implemented.
>>>> 1. The current state of affairs was to throw "NullArgumentException"; in 4.0
>>>>    this exception will become a subclass of NPE and we can continue to use
>>>>    it in the same way as now (i.e. possibly providing additional localizable
>>>>    error messages).
>>>> 2. The alternative is to directly throw a standard NPE, but without any
>>>>    message, since it wouldn't be localizable with the support of CM's
>>>>    "context".
>>>>
>>>> So, which of those alternatives do people want to settle on?
>>>>
>>>>
>>>> Regards,
>>>> Gilles
>>>>
>>>> [1] As opposed to being passed on to another method.
>>>>
>>> I have another question: are we going to little by little remove all
>>> null checks that we feel are not absolutely necessary? I would not
>>> mind going through this cleaning phase while working on MATH-854.
>> I think we should wait to remove the existing null checks until
>> 4.0.  This is a significant behavior change, especially for the ones
>> that have been in place and documented since 1.x.  Removing the
>> checks and allowing NPEs to propagate will break apps that catch IAE.
>>
>> Phil
>>
> OK. I would like to say that I think this is going the right way: it
> is better to have no null checks than incomplete null checks which
> provide a false sense of security. For example, have a look to
> Array2DRowRealMatrix
>
>
>     /**
>      * Create a new RealMatrix using the input array as the underlying
>      * data array.
>      * If an array is built specially in order to be embedded in a
>      * RealMatrix and not used directly, the {@code copyArray} may be
>      * set to {@code false}. This will prevent the copying and improve
>      * performance as no new array will be built and no data will be copied.
>      *
>      * @param d Data for new matrix.
>      * @param copyArray if {@code true}, the input array will be copied,
>      * otherwise it will be referenced.
>      * @throws DimensionMismatchException if {@code d} is not rectangular
>      * (not all rows have the same length) or empty.
>      * @throws NullArgumentException if {@code d} is {@code null}.
>      * @throws NoDataException if there are not at least one row and one column.
>      * @see #Array2DRowRealMatrix(double[][])
>      */
>     public Array2DRowRealMatrix(final double[][] d, final boolean copyArray) {
>         if (copyArray) {
>             copyIn(d);
>         } else {
>             if (d == null) {
>                 throw new NullArgumentException();
>             }
>             final int nRows = d.length;
>             if (nRows == 0) {
>                 throw new NoDataException(LocalizedFormats.AT_LEAST_ONE_ROW);
>             }
>             final int nCols = d[0].length;
>             if (nCols == 0) {
>                 throw new NoDataException(LocalizedFormats.AT_LEAST_ONE_COLUMN);
>             }
>             for (int r = 1; r < nRows; r++) {
>                 if (d[r].length != nCols) {
>                     throw new DimensionMismatchException(d[r].length, nCols);
>                 }
>             }
>             data = d;
>         }
>     }
>
> I think if you want to be really helpful, a null check should also be
> carried out on each d[i]. I think I will leave it as is for now, but
> in 4.0, we should certainly remove this null check.
> Do you agree?

Yes, the API should be nullsafe or not.  The half-way solution above
is no good and it makes the javadoc misleading.  I would be OK
actually adding the check at the row level now (or catching the NPE
and throwing DME in its place) so what the javadoc says is true (d
null means NAE and d non-rectangular or empty means DME).  On the
other hand, this is unlikely to ever happen in practice, so I would
leave as is for now.

Phil
> Sébastien
>
>
> ---------------------------------------------------------------------
> 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] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hi,

2012/9/18 Phil Steitz <ph...@gmail.com>:
> On 9/18/12 12:02 PM, Sébastien Brisard wrote:
>> Hello,
>>
>> 2012/9/17 Gilles Sadowski <gi...@harfang.homelinux.org>:
>>> On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote:
>>>> OK, I give up.  Lets do option 2.  Just warn users in the User Guide
>>>> somewhere that our APIs are in general not null-safe and unless the
>>>> javadoc explicitly allows nulls, they can expect NPE when passing nulls.
>>> Thanks, Phil; we are making progress, and I hope that you'll be convinced of
>>> that at some point.
>>>
>>> Another decision still needs to be made.
>>>
>>> I think that everybody now agrees that wherever an argument will be directly
>>> used (i.e. "dereferenced") inside the body of the method[1], it is safe to
>>> not check for null (since the JVM will throw an NPE).
>>>
>>> But, whenever an argument is passed for _later_ use (e.g. stored by a
>>> constructor or passed to another method), we also all expressed that it is
>>> better to fail early if the object is supposed to be non-null. In those
>>> cases, checks are not mandatory (since the JVM will throw NPE at some
>>> point), but we must agree on how optional checks are to be implemented.
>>> 1. The current state of affairs was to throw "NullArgumentException"; in 4.0
>>>    this exception will become a subclass of NPE and we can continue to use
>>>    it in the same way as now (i.e. possibly providing additional localizable
>>>    error messages).
>>> 2. The alternative is to directly throw a standard NPE, but without any
>>>    message, since it wouldn't be localizable with the support of CM's
>>>    "context".
>>>
>>> So, which of those alternatives do people want to settle on?
>>>
>>>
>>> Regards,
>>> Gilles
>>>
>>> [1] As opposed to being passed on to another method.
>>>
>> I have another question: are we going to little by little remove all
>> null checks that we feel are not absolutely necessary? I would not
>> mind going through this cleaning phase while working on MATH-854.
>
> I think we should wait to remove the existing null checks until
> 4.0.  This is a significant behavior change, especially for the ones
> that have been in place and documented since 1.x.  Removing the
> checks and allowing NPEs to propagate will break apps that catch IAE.
>
> Phil
>
OK. I would like to say that I think this is going the right way: it
is better to have no null checks than incomplete null checks which
provide a false sense of security. For example, have a look to
Array2DRowRealMatrix


    /**
     * Create a new RealMatrix using the input array as the underlying
     * data array.
     * If an array is built specially in order to be embedded in a
     * RealMatrix and not used directly, the {@code copyArray} may be
     * set to {@code false}. This will prevent the copying and improve
     * performance as no new array will be built and no data will be copied.
     *
     * @param d Data for new matrix.
     * @param copyArray if {@code true}, the input array will be copied,
     * otherwise it will be referenced.
     * @throws DimensionMismatchException if {@code d} is not rectangular
     * (not all rows have the same length) or empty.
     * @throws NullArgumentException if {@code d} is {@code null}.
     * @throws NoDataException if there are not at least one row and one column.
     * @see #Array2DRowRealMatrix(double[][])
     */
    public Array2DRowRealMatrix(final double[][] d, final boolean copyArray) {
        if (copyArray) {
            copyIn(d);
        } else {
            if (d == null) {
                throw new NullArgumentException();
            }
            final int nRows = d.length;
            if (nRows == 0) {
                throw new NoDataException(LocalizedFormats.AT_LEAST_ONE_ROW);
            }
            final int nCols = d[0].length;
            if (nCols == 0) {
                throw new NoDataException(LocalizedFormats.AT_LEAST_ONE_COLUMN);
            }
            for (int r = 1; r < nRows; r++) {
                if (d[r].length != nCols) {
                    throw new DimensionMismatchException(d[r].length, nCols);
                }
            }
            data = d;
        }
    }

I think if you want to be really helpful, a null check should also be
carried out on each d[i]. I think I will leave it as is for now, but
in 4.0, we should certainly remove this null check.
Do you agree?
Sébastien


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/18/12 12:02 PM, Sébastien Brisard wrote:
> Hello,
>
> 2012/9/17 Gilles Sadowski <gi...@harfang.homelinux.org>:
>> On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote:
>>> OK, I give up.  Lets do option 2.  Just warn users in the User Guide
>>> somewhere that our APIs are in general not null-safe and unless the
>>> javadoc explicitly allows nulls, they can expect NPE when passing nulls.
>> Thanks, Phil; we are making progress, and I hope that you'll be convinced of
>> that at some point.
>>
>> Another decision still needs to be made.
>>
>> I think that everybody now agrees that wherever an argument will be directly
>> used (i.e. "dereferenced") inside the body of the method[1], it is safe to
>> not check for null (since the JVM will throw an NPE).
>>
>> But, whenever an argument is passed for _later_ use (e.g. stored by a
>> constructor or passed to another method), we also all expressed that it is
>> better to fail early if the object is supposed to be non-null. In those
>> cases, checks are not mandatory (since the JVM will throw NPE at some
>> point), but we must agree on how optional checks are to be implemented.
>> 1. The current state of affairs was to throw "NullArgumentException"; in 4.0
>>    this exception will become a subclass of NPE and we can continue to use
>>    it in the same way as now (i.e. possibly providing additional localizable
>>    error messages).
>> 2. The alternative is to directly throw a standard NPE, but without any
>>    message, since it wouldn't be localizable with the support of CM's
>>    "context".
>>
>> So, which of those alternatives do people want to settle on?
>>
>>
>> Regards,
>> Gilles
>>
>> [1] As opposed to being passed on to another method.
>>
> I have another question: are we going to little by little remove all
> null checks that we feel are not absolutely necessary? I would not
> mind going through this cleaning phase while working on MATH-854.

I think we should wait to remove the existing null checks until
4.0.  This is a significant behavior change, especially for the ones
that have been in place and documented since 1.x.  Removing the
checks and allowing NPEs to propagate will break apps that catch IAE.

Phil
>
> Best regards,
> Sébastien
>
>
> ---------------------------------------------------------------------
> 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] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hello,

2012/9/17 Gilles Sadowski <gi...@harfang.homelinux.org>:
> On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote:
>> OK, I give up.  Lets do option 2.  Just warn users in the User Guide
>> somewhere that our APIs are in general not null-safe and unless the
>> javadoc explicitly allows nulls, they can expect NPE when passing nulls.
>
> Thanks, Phil; we are making progress, and I hope that you'll be convinced of
> that at some point.
>
> Another decision still needs to be made.
>
> I think that everybody now agrees that wherever an argument will be directly
> used (i.e. "dereferenced") inside the body of the method[1], it is safe to
> not check for null (since the JVM will throw an NPE).
>
> But, whenever an argument is passed for _later_ use (e.g. stored by a
> constructor or passed to another method), we also all expressed that it is
> better to fail early if the object is supposed to be non-null. In those
> cases, checks are not mandatory (since the JVM will throw NPE at some
> point), but we must agree on how optional checks are to be implemented.
> 1. The current state of affairs was to throw "NullArgumentException"; in 4.0
>    this exception will become a subclass of NPE and we can continue to use
>    it in the same way as now (i.e. possibly providing additional localizable
>    error messages).
> 2. The alternative is to directly throw a standard NPE, but without any
>    message, since it wouldn't be localizable with the support of CM's
>    "context".
>
> So, which of those alternatives do people want to settle on?
>
>
> Regards,
> Gilles
>
> [1] As opposed to being passed on to another method.
>
I have another question: are we going to little by little remove all
null checks that we feel are not absolutely necessary? I would not
mind going through this cleaning phase while working on MATH-854.

Best regards,
Sébastien


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote:
> OK, I give up.  Lets do option 2.  Just warn users in the User Guide
> somewhere that our APIs are in general not null-safe and unless the
> javadoc explicitly allows nulls, they can expect NPE when passing nulls.

Thanks, Phil; we are making progress, and I hope that you'll be convinced of
that at some point.

Another decision still needs to be made.

I think that everybody now agrees that wherever an argument will be directly
used (i.e. "dereferenced") inside the body of the method[1], it is safe to
not check for null (since the JVM will throw an NPE).

But, whenever an argument is passed for _later_ use (e.g. stored by a
constructor or passed to another method), we also all expressed that it is
better to fail early if the object is supposed to be non-null. In those
cases, checks are not mandatory (since the JVM will throw NPE at some
point), but we must agree on how optional checks are to be implemented.
1. The current state of affairs was to throw "NullArgumentException"; in 4.0
   this exception will become a subclass of NPE and we can continue to use
   it in the same way as now (i.e. possibly providing additional localizable
   error messages).
2. The alternative is to directly throw a standard NPE, but without any
   message, since it wouldn't be localizable with the support of CM's
   "context".

So, which of those alternatives do people want to settle on?


Regards,
Gilles

[1] As opposed to being passed on to another method.

> Phil
> 
> On 9/14/12 8:46 AM, Gilles Sadowski wrote:
> > On Fri, Sep 14, 2012 at 07:44:08AM -0700, Phil Steitz wrote:
> >> On 9/14/12 4:28 AM, Gilles Sadowski wrote:
> >>> On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote:
> >>>> Le 14/09/2012 08:46, Sébastien Brisard a écrit :
> >>>>> Hi Phil
> >>>>>
> >>>>>>> Back to square one, with 3 fully consistent alternatives:
> >>>>>>>  1. CM to always check for null? Then "NullArgumentException" inheriting from
> >>>>>>>     "MathIllegalArgumentException" is fine because we promise to the user that
> >>>>>>>     no NPE will ever propagate through the CM layer. [Breaking that promise
> >>>>>>>     is a CM bug.]
> >>>>>>>  2. CM to never check for null? Then we delete class "NullArgumentException".
> >>>>>>>     Users are warned by the general policy: "Do not pass null unless it is
> >>>>>>>     explicitly documented to be allowed." A bug will lead to the JVM raising
> >>>>>>>     a NPE.
> >>>>>>>  3. CM to sometimes check for null? Then "NullArgumentException" should
> >>>>>>>     inherit from "NullPointerException" because the user will sometimes see
> >>>>>>>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
> >>>>>>>     not check) and both should thus belong to the same hierarchy (from the
> >>>>>>>     user's point-of-view, there is no reason to handle them in different
> >>>>>>>     ways since it's the exact same bug).
> >>>>>>>     For the user, the consequence will be similar to alternative 2, with
> >>>>>>>     sometimes more information about the failure and sometimes (marginally)
> >>>>>>>     earlier detection.
> >>>>>> As stated above, my preference is
> >>>>>>
> >>>>>> 4.  CM APIs *may* disallow nulls explicitly.  When that is done, the
> >>>>>> implementations *should* check parameters (as they *should* check
> >>>>>> all other stated preconditions) and when preconditions are violated,
> >>>>>> a MathIllegalArgumentException is thrown.  I don't care whether or
> >>>>>> not we keep NAE.  If we drop it, we should make sure whatever
> >>>>>> exception messages we used to use to indicate illegal null arguments
> >>>>>> are still there.
> >>>>>>
> >>>>>> Phil
> >>>>>>
> >>>>> I like this option better than 3. So, I'll change my "vote" to option
> >>>>> #2, and possibly option #4 as we will never agree on option #2.
> >>> Why is it better to throw MathIllegalArgumentException rather than NPE?
> >> Because, as I have repeated numerous times, that is the appropriate
> >> exception to throw when API preconditions are violated.  That is why
> >> IAE exists.  The class javadoc comment for IAE is nice and succinct
> >> (we could learn from it ;)
> > IllegalArgumentException:
> > ---
> > Thrown to indicate that a method has been passed an illegal or inappropriate
> > argument. 
> > ---
> >
> > Did you read the class Javadoc for NPE?
> > Here is the last sentence pasted again:
> > ---
> > Applications should throw instances of this class to indicate other illegal
> > uses of the null object. 
> > ---
> >
> >>   You could ask the same question about
> >> ArrayIndexOutOfBounds.  Why not throw that instead of IAE when bad
> >> indices are provided?
> > Does CM check _array_ index? No.
> > Surprised? An "ArrayRealVector" is _not_ an array, it is the representation
> > of the vector concept. That it is a backed by a Java array is an implemtation
> > detail that should not surface to the API. Hence the choice to not use the
> > low-level ArrayIndexOutOfBoundsException can be given a rationale.
> > CM checks that you access valid entries of the high-level "mathematical"
> > object. And we do that for _all_ such accesses and never (or it is a CM bug)
> > let propagate an exception that reveals an underlying Java array.
> > [This was one of my proposals too (alternative 1). However it imposes a
> > unnecessary burden of duplicating (CM and JVM) every null check just for the
> > sake of hiding NPE.]
> >
> > As indicated in a previous post, "null" is not a "RealVector", it is not an
> > "Object", it is not a reference, it is just the value of an unitialized
> > pointer.
> >
> >>   The difference is that instead of letting
> >> the precondition failure go undetected and the runtime exception
> >> propagate, APIs with explicit preconditions and parameter checking
> >> in place raise the exception at method activation time.
> > Could you please stop mentioning this, since my proposal (alternative 3)
> > allows for early detection?
> >
> > The difference is that you want to use MathIAE, where everyone else uses NPE.
> >
> >>  In that
> >> context, what is appropriate is IAE.
> > You are stuck with a "name", as if everything that is "illegal" must be
> > signalled with an exception that contains the string "Illegal" in its name.
> >
> > If that is so, I propose to rename "NullArgumentException" to
> > "IllegalNullArgumentException".
> >
> >
> > Gilles
> >
> >> Phil
> >>>> My preferences are 4 or 3 with same preference level and 2 as a fallback
> >>>> choice. For each option, I find the arguments are good, it's only a
> >>>> matter or preference.
> >>> Did everyone consider that Phil's alternative implies a behaviour that
> >>> departs from the standard practice, a.o. 
> >>>  * JDK (cf. quote in a previous post),
> >>>  * other libraries, e.g. Guava (cf. quote in a previous post), and
> >>>  * Java experts, e.g. Joshua Bloch[1]
> >>> of throwing NPE when they encounter "null"?
> >>> Doesn't anyone care about having a _reason_ for CM to behave differently?
> >>>
> >>> Don't you care that users will see different exceptions when the same
> >>> problem is detected?
> >>>
> >>> Do you really like a policy that mandates different behaviours when "null"
> >>> is disallowed implicitly vs explicitly (throwing NPE vs throwing
> >>> MathRuntimeException)?  [Pardon me, but IMHO this is nonsense.]
> >>>
> >>> After piling up arguments (_none_ of which have been addressed), I'm sorry
> >>> to read that it would only be a "matter of preference"! [I should start a
> >>> career as a writer if I'm able to express "preference" in so many words...]
> >>> Actually, it's a matter of consistency ("same problem, same exception").
> >>> If nobody cares about improving CM's consistency, then indeed most of the
> >>> discussions in which I take part are pointless.
> >>>
> >>> What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows
> >>> CM to fail as early as possible (which I thought was Phil's main point, as
> >>> it was his only one, apart from "preference"), while being consistent ("same
> >>> problem, same exception"), internally, externally, implicitly and
> >>> explicitly. Alternative 4 is inconsistent ("same problem, different
> >>> exceptions"); it's a truth, not a matter of taste.]
> >>>
> >>> Rather then being willingly stuck in a deadlock because only the weakest
> >>> argument ("preference") is taken into account, wouldn't it be more
> >>> reasonable to count all the arguments?
> >>>
> >>>
> >>> Gilles
> >>>
> >>> [1] "Effective Java" (second edition). Excerpt from "Item 60":
> >>>     If a caller passes null in some parameter for which null values are
> >>>     prohibited, convention dictates that NullPointerException be thrown
> >>>     rather than IllegalArgumentException.
> >>>
> >>>> Luc
> >>>>
> >>>>> Best regards,
> >>>>> Sébastien
> >>>>>>> Your alternative (sometimes check, sometimes not) is not fully consistent:
> >>>>>>>  * for the user: "In case of null reference, will I get a MathRuntimeException
> >>>>>>>    or a NPE?"
> >>>>>>>  * for the CM developer: "In which cases do I need to check for null?"
> >>>>>>>
> >>>>>>> Of course, I would reconsider if you could provide an actual example that
> >>>>>>> would fail with all three alternatives which I suggested. If not, then
> >>>>>>> it seems obvious that your alternative presents two defects that don't exist
> >>>>>>> in any of those three.
> >>>>>>>
> >>>>>>>
> >>>>>>> Gilles
> >>>>>>>
> >>>>>>>>>> [...] what is different about null arguments at the point of
> >>>>>>>>>> method activation in an API that explicitly disallows them.
> >>>>>>>>> The difference is that there is no need to tell the user what the problem
> >>>>>>>>> is because the raised exception says it all: "You tried to use a null
> >>>>>>>>> reference." [As said above, the only issue is _when_ the exception is
> >>>>>>>>> triggered.]
> >>>>>>>>>
> >>>>>>>>> The policy mandates what is globally valid, e.g.:
> >>>>>>>>>   "If not specified otherwise, "null" is not allowed as an argument."
> >>>>>>>>> Then, a user cannot complain about a NPE being propagated when he passed an
> >>>>>>>>> invalid (null) argument.
> >>>>>>>>>
> >>>>>>>>> Finally, the case for "null" is also slightly peculiar (given the above)
> >>>>>>>>> that it should not simply be bundled with the rationale "Fail early": Indeed
> >>>>>>>>> "null" references always fail early (i.e. as soon as they are used).
> >>>>>>>>> Deferring the check until it is done by the JVM will never entails the code
> >>>>>>>>> to produce a wrong result _because_ of that null reference (it will just
> >>>>>>>>> fail in the predictable way: NPE).[1]
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Gilles
> >>>>>>>>>
> >>>>>>>>> [1] Unlike in C, where an unitialized pointer would not necessarily crash
> >>>>>>>>>     the program, but _could_ make it behave erratically (including produce
> >>>>>>>>>     wrong results in a stealth way).
> >>>>>>>>>
> > ---------------------------------------------------------------------
> > 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] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
OK, I give up.  Lets do option 2.  Just warn users in the User Guide
somewhere that our APIs are in general not null-safe and unless the
javadoc explicitly allows nulls, they can expect NPE when passing nulls.

Phil

On 9/14/12 8:46 AM, Gilles Sadowski wrote:
> On Fri, Sep 14, 2012 at 07:44:08AM -0700, Phil Steitz wrote:
>> On 9/14/12 4:28 AM, Gilles Sadowski wrote:
>>> On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote:
>>>> Le 14/09/2012 08:46, Sébastien Brisard a écrit :
>>>>> Hi Phil
>>>>>
>>>>>>> Back to square one, with 3 fully consistent alternatives:
>>>>>>>  1. CM to always check for null? Then "NullArgumentException" inheriting from
>>>>>>>     "MathIllegalArgumentException" is fine because we promise to the user that
>>>>>>>     no NPE will ever propagate through the CM layer. [Breaking that promise
>>>>>>>     is a CM bug.]
>>>>>>>  2. CM to never check for null? Then we delete class "NullArgumentException".
>>>>>>>     Users are warned by the general policy: "Do not pass null unless it is
>>>>>>>     explicitly documented to be allowed." A bug will lead to the JVM raising
>>>>>>>     a NPE.
>>>>>>>  3. CM to sometimes check for null? Then "NullArgumentException" should
>>>>>>>     inherit from "NullPointerException" because the user will sometimes see
>>>>>>>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
>>>>>>>     not check) and both should thus belong to the same hierarchy (from the
>>>>>>>     user's point-of-view, there is no reason to handle them in different
>>>>>>>     ways since it's the exact same bug).
>>>>>>>     For the user, the consequence will be similar to alternative 2, with
>>>>>>>     sometimes more information about the failure and sometimes (marginally)
>>>>>>>     earlier detection.
>>>>>> As stated above, my preference is
>>>>>>
>>>>>> 4.  CM APIs *may* disallow nulls explicitly.  When that is done, the
>>>>>> implementations *should* check parameters (as they *should* check
>>>>>> all other stated preconditions) and when preconditions are violated,
>>>>>> a MathIllegalArgumentException is thrown.  I don't care whether or
>>>>>> not we keep NAE.  If we drop it, we should make sure whatever
>>>>>> exception messages we used to use to indicate illegal null arguments
>>>>>> are still there.
>>>>>>
>>>>>> Phil
>>>>>>
>>>>> I like this option better than 3. So, I'll change my "vote" to option
>>>>> #2, and possibly option #4 as we will never agree on option #2.
>>> Why is it better to throw MathIllegalArgumentException rather than NPE?
>> Because, as I have repeated numerous times, that is the appropriate
>> exception to throw when API preconditions are violated.  That is why
>> IAE exists.  The class javadoc comment for IAE is nice and succinct
>> (we could learn from it ;)
> IllegalArgumentException:
> ---
> Thrown to indicate that a method has been passed an illegal or inappropriate
> argument. 
> ---
>
> Did you read the class Javadoc for NPE?
> Here is the last sentence pasted again:
> ---
> Applications should throw instances of this class to indicate other illegal
> uses of the null object. 
> ---
>
>>   You could ask the same question about
>> ArrayIndexOutOfBounds.  Why not throw that instead of IAE when bad
>> indices are provided?
> Does CM check _array_ index? No.
> Surprised? An "ArrayRealVector" is _not_ an array, it is the representation
> of the vector concept. That it is a backed by a Java array is an implemtation
> detail that should not surface to the API. Hence the choice to not use the
> low-level ArrayIndexOutOfBoundsException can be given a rationale.
> CM checks that you access valid entries of the high-level "mathematical"
> object. And we do that for _all_ such accesses and never (or it is a CM bug)
> let propagate an exception that reveals an underlying Java array.
> [This was one of my proposals too (alternative 1). However it imposes a
> unnecessary burden of duplicating (CM and JVM) every null check just for the
> sake of hiding NPE.]
>
> As indicated in a previous post, "null" is not a "RealVector", it is not an
> "Object", it is not a reference, it is just the value of an unitialized
> pointer.
>
>>   The difference is that instead of letting
>> the precondition failure go undetected and the runtime exception
>> propagate, APIs with explicit preconditions and parameter checking
>> in place raise the exception at method activation time.
> Could you please stop mentioning this, since my proposal (alternative 3)
> allows for early detection?
>
> The difference is that you want to use MathIAE, where everyone else uses NPE.
>
>>  In that
>> context, what is appropriate is IAE.
> You are stuck with a "name", as if everything that is "illegal" must be
> signalled with an exception that contains the string "Illegal" in its name.
>
> If that is so, I propose to rename "NullArgumentException" to
> "IllegalNullArgumentException".
>
>
> Gilles
>
>> Phil
>>>> My preferences are 4 or 3 with same preference level and 2 as a fallback
>>>> choice. For each option, I find the arguments are good, it's only a
>>>> matter or preference.
>>> Did everyone consider that Phil's alternative implies a behaviour that
>>> departs from the standard practice, a.o. 
>>>  * JDK (cf. quote in a previous post),
>>>  * other libraries, e.g. Guava (cf. quote in a previous post), and
>>>  * Java experts, e.g. Joshua Bloch[1]
>>> of throwing NPE when they encounter "null"?
>>> Doesn't anyone care about having a _reason_ for CM to behave differently?
>>>
>>> Don't you care that users will see different exceptions when the same
>>> problem is detected?
>>>
>>> Do you really like a policy that mandates different behaviours when "null"
>>> is disallowed implicitly vs explicitly (throwing NPE vs throwing
>>> MathRuntimeException)?  [Pardon me, but IMHO this is nonsense.]
>>>
>>> After piling up arguments (_none_ of which have been addressed), I'm sorry
>>> to read that it would only be a "matter of preference"! [I should start a
>>> career as a writer if I'm able to express "preference" in so many words...]
>>> Actually, it's a matter of consistency ("same problem, same exception").
>>> If nobody cares about improving CM's consistency, then indeed most of the
>>> discussions in which I take part are pointless.
>>>
>>> What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows
>>> CM to fail as early as possible (which I thought was Phil's main point, as
>>> it was his only one, apart from "preference"), while being consistent ("same
>>> problem, same exception"), internally, externally, implicitly and
>>> explicitly. Alternative 4 is inconsistent ("same problem, different
>>> exceptions"); it's a truth, not a matter of taste.]
>>>
>>> Rather then being willingly stuck in a deadlock because only the weakest
>>> argument ("preference") is taken into account, wouldn't it be more
>>> reasonable to count all the arguments?
>>>
>>>
>>> Gilles
>>>
>>> [1] "Effective Java" (second edition). Excerpt from "Item 60":
>>>     If a caller passes null in some parameter for which null values are
>>>     prohibited, convention dictates that NullPointerException be thrown
>>>     rather than IllegalArgumentException.
>>>
>>>> Luc
>>>>
>>>>> Best regards,
>>>>> Sébastien
>>>>>>> Your alternative (sometimes check, sometimes not) is not fully consistent:
>>>>>>>  * for the user: "In case of null reference, will I get a MathRuntimeException
>>>>>>>    or a NPE?"
>>>>>>>  * for the CM developer: "In which cases do I need to check for null?"
>>>>>>>
>>>>>>> Of course, I would reconsider if you could provide an actual example that
>>>>>>> would fail with all three alternatives which I suggested. If not, then
>>>>>>> it seems obvious that your alternative presents two defects that don't exist
>>>>>>> in any of those three.
>>>>>>>
>>>>>>>
>>>>>>> Gilles
>>>>>>>
>>>>>>>>>> [...] what is different about null arguments at the point of
>>>>>>>>>> method activation in an API that explicitly disallows them.
>>>>>>>>> The difference is that there is no need to tell the user what the problem
>>>>>>>>> is because the raised exception says it all: "You tried to use a null
>>>>>>>>> reference." [As said above, the only issue is _when_ the exception is
>>>>>>>>> triggered.]
>>>>>>>>>
>>>>>>>>> The policy mandates what is globally valid, e.g.:
>>>>>>>>>   "If not specified otherwise, "null" is not allowed as an argument."
>>>>>>>>> Then, a user cannot complain about a NPE being propagated when he passed an
>>>>>>>>> invalid (null) argument.
>>>>>>>>>
>>>>>>>>> Finally, the case for "null" is also slightly peculiar (given the above)
>>>>>>>>> that it should not simply be bundled with the rationale "Fail early": Indeed
>>>>>>>>> "null" references always fail early (i.e. as soon as they are used).
>>>>>>>>> Deferring the check until it is done by the JVM will never entails the code
>>>>>>>>> to produce a wrong result _because_ of that null reference (it will just
>>>>>>>>> fail in the predictable way: NPE).[1]
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Gilles
>>>>>>>>>
>>>>>>>>> [1] Unlike in C, where an unitialized pointer would not necessarily crash
>>>>>>>>>     the program, but _could_ make it behave erratically (including produce
>>>>>>>>>     wrong results in a stealth way).
>>>>>>>>>
> ---------------------------------------------------------------------
> 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] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Fri, Sep 14, 2012 at 07:44:08AM -0700, Phil Steitz wrote:
> On 9/14/12 4:28 AM, Gilles Sadowski wrote:
> > On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote:
> >> Le 14/09/2012 08:46, Sébastien Brisard a écrit :
> >>> Hi Phil
> >>>
> >>>>> Back to square one, with 3 fully consistent alternatives:
> >>>>>  1. CM to always check for null? Then "NullArgumentException" inheriting from
> >>>>>     "MathIllegalArgumentException" is fine because we promise to the user that
> >>>>>     no NPE will ever propagate through the CM layer. [Breaking that promise
> >>>>>     is a CM bug.]
> >>>>>  2. CM to never check for null? Then we delete class "NullArgumentException".
> >>>>>     Users are warned by the general policy: "Do not pass null unless it is
> >>>>>     explicitly documented to be allowed." A bug will lead to the JVM raising
> >>>>>     a NPE.
> >>>>>  3. CM to sometimes check for null? Then "NullArgumentException" should
> >>>>>     inherit from "NullPointerException" because the user will sometimes see
> >>>>>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
> >>>>>     not check) and both should thus belong to the same hierarchy (from the
> >>>>>     user's point-of-view, there is no reason to handle them in different
> >>>>>     ways since it's the exact same bug).
> >>>>>     For the user, the consequence will be similar to alternative 2, with
> >>>>>     sometimes more information about the failure and sometimes (marginally)
> >>>>>     earlier detection.
> >>>> As stated above, my preference is
> >>>>
> >>>> 4.  CM APIs *may* disallow nulls explicitly.  When that is done, the
> >>>> implementations *should* check parameters (as they *should* check
> >>>> all other stated preconditions) and when preconditions are violated,
> >>>> a MathIllegalArgumentException is thrown.  I don't care whether or
> >>>> not we keep NAE.  If we drop it, we should make sure whatever
> >>>> exception messages we used to use to indicate illegal null arguments
> >>>> are still there.
> >>>>
> >>>> Phil
> >>>>
> >>> I like this option better than 3. So, I'll change my "vote" to option
> >>> #2, and possibly option #4 as we will never agree on option #2.
> > Why is it better to throw MathIllegalArgumentException rather than NPE?
> 
> Because, as I have repeated numerous times, that is the appropriate
> exception to throw when API preconditions are violated.  That is why
> IAE exists.  The class javadoc comment for IAE is nice and succinct
> (we could learn from it ;)

IllegalArgumentException:
---
Thrown to indicate that a method has been passed an illegal or inappropriate
argument. 
---

Did you read the class Javadoc for NPE?
Here is the last sentence pasted again:
---
Applications should throw instances of this class to indicate other illegal
uses of the null object. 
---

>   You could ask the same question about
> ArrayIndexOutOfBounds.  Why not throw that instead of IAE when bad
> indices are provided?

Does CM check _array_ index? No.
Surprised? An "ArrayRealVector" is _not_ an array, it is the representation
of the vector concept. That it is a backed by a Java array is an implemtation
detail that should not surface to the API. Hence the choice to not use the
low-level ArrayIndexOutOfBoundsException can be given a rationale.
CM checks that you access valid entries of the high-level "mathematical"
object. And we do that for _all_ such accesses and never (or it is a CM bug)
let propagate an exception that reveals an underlying Java array.
[This was one of my proposals too (alternative 1). However it imposes a
unnecessary burden of duplicating (CM and JVM) every null check just for the
sake of hiding NPE.]

As indicated in a previous post, "null" is not a "RealVector", it is not an
"Object", it is not a reference, it is just the value of an unitialized
pointer.

>   The difference is that instead of letting
> the precondition failure go undetected and the runtime exception
> propagate, APIs with explicit preconditions and parameter checking
> in place raise the exception at method activation time.

Could you please stop mentioning this, since my proposal (alternative 3)
allows for early detection?

The difference is that you want to use MathIAE, where everyone else uses NPE.

>  In that
> context, what is appropriate is IAE.

You are stuck with a "name", as if everything that is "illegal" must be
signalled with an exception that contains the string "Illegal" in its name.

If that is so, I propose to rename "NullArgumentException" to
"IllegalNullArgumentException".


Gilles

> 
> Phil
> >
> >> My preferences are 4 or 3 with same preference level and 2 as a fallback
> >> choice. For each option, I find the arguments are good, it's only a
> >> matter or preference.
> > Did everyone consider that Phil's alternative implies a behaviour that
> > departs from the standard practice, a.o. 
> >  * JDK (cf. quote in a previous post),
> >  * other libraries, e.g. Guava (cf. quote in a previous post), and
> >  * Java experts, e.g. Joshua Bloch[1]
> > of throwing NPE when they encounter "null"?
> > Doesn't anyone care about having a _reason_ for CM to behave differently?
> >
> > Don't you care that users will see different exceptions when the same
> > problem is detected?
> >
> > Do you really like a policy that mandates different behaviours when "null"
> > is disallowed implicitly vs explicitly (throwing NPE vs throwing
> > MathRuntimeException)?  [Pardon me, but IMHO this is nonsense.]
> >
> > After piling up arguments (_none_ of which have been addressed), I'm sorry
> > to read that it would only be a "matter of preference"! [I should start a
> > career as a writer if I'm able to express "preference" in so many words...]
> > Actually, it's a matter of consistency ("same problem, same exception").
> > If nobody cares about improving CM's consistency, then indeed most of the
> > discussions in which I take part are pointless.
> >
> > What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows
> > CM to fail as early as possible (which I thought was Phil's main point, as
> > it was his only one, apart from "preference"), while being consistent ("same
> > problem, same exception"), internally, externally, implicitly and
> > explicitly. Alternative 4 is inconsistent ("same problem, different
> > exceptions"); it's a truth, not a matter of taste.]
> >
> > Rather then being willingly stuck in a deadlock because only the weakest
> > argument ("preference") is taken into account, wouldn't it be more
> > reasonable to count all the arguments?
> >
> >
> > Gilles
> >
> > [1] "Effective Java" (second edition). Excerpt from "Item 60":
> >     If a caller passes null in some parameter for which null values are
> >     prohibited, convention dictates that NullPointerException be thrown
> >     rather than IllegalArgumentException.
> >
> >> Luc
> >>
> >>> Best regards,
> >>> Sébastien
> >>>>> Your alternative (sometimes check, sometimes not) is not fully consistent:
> >>>>>  * for the user: "In case of null reference, will I get a MathRuntimeException
> >>>>>    or a NPE?"
> >>>>>  * for the CM developer: "In which cases do I need to check for null?"
> >>>>>
> >>>>> Of course, I would reconsider if you could provide an actual example that
> >>>>> would fail with all three alternatives which I suggested. If not, then
> >>>>> it seems obvious that your alternative presents two defects that don't exist
> >>>>> in any of those three.
> >>>>>
> >>>>>
> >>>>> Gilles
> >>>>>
> >>>>>>>> [...] what is different about null arguments at the point of
> >>>>>>>> method activation in an API that explicitly disallows them.
> >>>>>>> The difference is that there is no need to tell the user what the problem
> >>>>>>> is because the raised exception says it all: "You tried to use a null
> >>>>>>> reference." [As said above, the only issue is _when_ the exception is
> >>>>>>> triggered.]
> >>>>>>>
> >>>>>>> The policy mandates what is globally valid, e.g.:
> >>>>>>>   "If not specified otherwise, "null" is not allowed as an argument."
> >>>>>>> Then, a user cannot complain about a NPE being propagated when he passed an
> >>>>>>> invalid (null) argument.
> >>>>>>>
> >>>>>>> Finally, the case for "null" is also slightly peculiar (given the above)
> >>>>>>> that it should not simply be bundled with the rationale "Fail early": Indeed
> >>>>>>> "null" references always fail early (i.e. as soon as they are used).
> >>>>>>> Deferring the check until it is done by the JVM will never entails the code
> >>>>>>> to produce a wrong result _because_ of that null reference (it will just
> >>>>>>> fail in the predictable way: NPE).[1]
> >>>>>>>
> >>>>>>>
> >>>>>>> Gilles
> >>>>>>>
> >>>>>>> [1] Unlike in C, where an unitialized pointer would not necessarily crash
> >>>>>>>     the program, but _could_ make it behave erratically (including produce
> >>>>>>>     wrong results in a stealth way).
> >>>>>>>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.

On Thu, Sep 13, 2012 at 01:34:38PM -0400, Kish, Robert wrote:
> Hello - 
> 	I've been following this mailing list for a few months, and I
> feel this is a good opportunity to provide a first opinion on a topic.
> 
> I find option 2, to never check for null, the best option. But, I think
> it would work even better with annotations such as package level
> @NonNullByDefault, which is what CM appears to follow. For functions
> that allow null, they could be tagged @Nullable.
> 
> These annotations have been made more popular by Eclipse Juno including
> easier support for them, and I find in my own code, they are nice to
> use. When using these annotations, any superfluous null checks inside
> the CM library would trigger a compiler warning that they're not needed.
> 
> Any CM functions being called externally would be flagged that they
> don't allow null values to be passed in.
> 
> In theory, we'd have compile time null checking, so no NPE at all.
> 
> Note that I'm not suggesting that these annotations be used now, but if
> they were to be used one day, there would be little changes needed to CM
> code (no compiler warnings to remove due to unnecessary null checks).

Thanks for this input.
I'd also think that using annotations is the next step for increased
consistency (i.e. a concise syntax that spares the developers from
repetitive bits of code and allowing automatic enforcement of various
constraints).

Unfortunately, that's not for CM at this time since it's still supposed to
be source-compatible with Java 5 and cannot depend from external libraries.
I'm not sure, but I think that we also cannot use the extensions of JavaEE
(?).

A temporary solution would be that we locally implement the annotation code
(?). Maybe we can copy/paste it from somewhere (?).


Regards,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hi,

2012/9/13 Gilles Sadowski <gi...@harfang.homelinux.org>:
> On Thu, Sep 13, 2012 at 01:10:41AM +0200, Gilles Sadowski wrote:
>> Hello.
>>
>> [For those who don't wish to read the whole post, please at least go towards
>> the end and indicate your preferred option. Thanks.]
>>
>> [... long post skipped ...]
>>
>> Back to square one, with 3 fully consistent alternatives:
>>  1. CM to always check for null? Then "NullArgumentException" inheriting from
>>     "MathIllegalArgumentException" is fine because we promise to the user that
>>     no NPE will ever propagate through the CM layer. [Breaking that promise
>>     is a CM bug.]
>>  2. CM to never check for null? Then we delete class "NullArgumentException".
>>     Users are warned by the general policy: "Do not pass null unless it is
>>     explicitly documented to be allowed." A bug will lead to the JVM raising
>>     a NPE.
>>  3. CM to sometimes check for null? Then "NullArgumentException" should
>>     inherit from "NullPointerException" because the user will sometimes see
>>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
>>     not check) and both should thus belong to the same hierarchy (from the
>>     user's point-of-view, there is no reason to handle them in different
>>     ways since it's the exact same bug).
>>     For the user, the consequence will be similar to alternative 2, with
>>     sometimes more information about the failure and sometimes (marginally)
>>     earlier detection.
>
> My preference would be alternative 2 but I'd be happy with alternative 3 too
> (as a compromise to allow checks for people who need more time to get rid
> of the habit :-).
>
I would also favor option #2. I can live with #3.

Sébastien

> With those additional arguments, for good measure:
>
> (a)
> ---
> Thrown when an application attempts to use null in a case where an object is
> required. These include:
>
>     Calling the instance method of a null object.
>     Accessing or modifying the field of a null object.
>     Taking the length of null as if it were an array.
>     Accessing or modifying the slots of null as if it were an array.
>     Throwing null as if it were a Throwable value.
>
> Applications should throw instances of this class to indicate other illegal
> uses of the null object.
> ---
> [This is the Javadoc for NPE. Please note the last sentence.]
>
> (b)
> "null" is not a representation of an application-domain concept, it is a low
> low low level literal (the unique element of the null-type) and exists solely
> to be the value of unitialized pointers (not even "reference"). It is only
> perfectly right that attempts to use "null" are set apart from other bugs
> (that acquire meaning only wrt to higher level preconditions).
>
> (c)
> http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/base/Preconditions.html#checkNotNull(T)
>
>
>
> 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] About "NullArgumentException"

Posted by "Kish, Robert" <Ro...@ncogroup.com>.
Hello - 
	I've been following this mailing list for a few months, and I
feel this is a good opportunity to provide a first opinion on a topic.

I find option 2, to never check for null, the best option. But, I think
it would work even better with annotations such as package level
@NonNullByDefault, which is what CM appears to follow. For functions
that allow null, they could be tagged @Nullable.

These annotations have been made more popular by Eclipse Juno including
easier support for them, and I find in my own code, they are nice to
use. When using these annotations, any superfluous null checks inside
the CM library would trigger a compiler warning that they're not needed.

Any CM functions being called externally would be flagged that they
don't allow null values to be passed in.

In theory, we'd have compile time null checking, so no NPE at all.

Note that I'm not suggesting that these annotations be used now, but if
they were to be used one day, there would be little changes needed to CM
code (no compiler warnings to remove due to unnecessary null checks).

Robert Kish


-----Original Message-----
From: Gilles Sadowski [mailto:gilles@harfang.homelinux.org] 
Sent: Thursday, September 13, 2012 10:36 AM
To: dev@commons.apache.org
Subject: Re: [Math] About "NullArgumentException"

On Thu, Sep 13, 2012 at 01:10:41AM +0200, Gilles Sadowski wrote:
> Hello.
> 
> [For those who don't wish to read the whole post, please at least go
towards
> the end and indicate your preferred option. Thanks.]
> 
> [... long post skipped ...]
> 
> Back to square one, with 3 fully consistent alternatives:
>  1. CM to always check for null? Then "NullArgumentException"
inheriting from
>     "MathIllegalArgumentException" is fine because we promise to the
user that
>     no NPE will ever propagate through the CM layer. [Breaking that
promise
>     is a CM bug.]
>  2. CM to never check for null? Then we delete class
"NullArgumentException".
>     Users are warned by the general policy: "Do not pass null unless
it is
>     explicitly documented to be allowed." A bug will lead to the JVM
raising
>     a NPE.
>  3. CM to sometimes check for null? Then "NullArgumentException"
should
>     inherit from "NullPointerException" because the user will
sometimes see
>     "NullArgumentException" (when CM checks) and sometimes NPE (when
CM does
>     not check) and both should thus belong to the same hierarchy (from
the
>     user's point-of-view, there is no reason to handle them in
different
>     ways since it's the exact same bug).
>     For the user, the consequence will be similar to alternative 2,
with
>     sometimes more information about the failure and sometimes
(marginally)
>     earlier detection.

My preference would be alternative 2 but I'd be happy with alternative 3
too
(as a compromise to allow checks for people who need more time to get
rid
of the habit :-).

<snip>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Thu, Sep 13, 2012 at 01:10:41AM +0200, Gilles Sadowski wrote:
> Hello.
> 
> [For those who don't wish to read the whole post, please at least go towards
> the end and indicate your preferred option. Thanks.]
> 
> [... long post skipped ...]
> 
> Back to square one, with 3 fully consistent alternatives:
>  1. CM to always check for null? Then "NullArgumentException" inheriting from
>     "MathIllegalArgumentException" is fine because we promise to the user that
>     no NPE will ever propagate through the CM layer. [Breaking that promise
>     is a CM bug.]
>  2. CM to never check for null? Then we delete class "NullArgumentException".
>     Users are warned by the general policy: "Do not pass null unless it is
>     explicitly documented to be allowed." A bug will lead to the JVM raising
>     a NPE.
>  3. CM to sometimes check for null? Then "NullArgumentException" should
>     inherit from "NullPointerException" because the user will sometimes see
>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
>     not check) and both should thus belong to the same hierarchy (from the
>     user's point-of-view, there is no reason to handle them in different
>     ways since it's the exact same bug).
>     For the user, the consequence will be similar to alternative 2, with
>     sometimes more information about the failure and sometimes (marginally)
>     earlier detection.

My preference would be alternative 2 but I'd be happy with alternative 3 too
(as a compromise to allow checks for people who need more time to get rid
of the habit :-).

With those additional arguments, for good measure:

(a)
---
Thrown when an application attempts to use null in a case where an object is
required. These include:

    Calling the instance method of a null object.
    Accessing or modifying the field of a null object.
    Taking the length of null as if it were an array.
    Accessing or modifying the slots of null as if it were an array.
    Throwing null as if it were a Throwable value. 

Applications should throw instances of this class to indicate other illegal
uses of the null object. 
---
[This is the Javadoc for NPE. Please note the last sentence.]

(b)
"null" is not a representation of an application-domain concept, it is a low
low low level literal (the unique element of the null-type) and exists solely
to be the value of unitialized pointers (not even "reference"). It is only
perfectly right that attempts to use "null" are set apart from other bugs
(that acquire meaning only wrt to higher level preconditions).

(c)
http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/base/Preconditions.html#checkNotNull(T)



Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hi Gilles,

2012/9/14 Gilles Sadowski <gi...@harfang.homelinux.org>:
> On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote:
>> Le 14/09/2012 08:46, Sébastien Brisard a écrit :
>> > Hi Phil
>> >
>> >>>
>> >>> Back to square one, with 3 fully consistent alternatives:
>> >>>  1. CM to always check for null? Then "NullArgumentException" inheriting from
>> >>>     "MathIllegalArgumentException" is fine because we promise to the user that
>> >>>     no NPE will ever propagate through the CM layer. [Breaking that promise
>> >>>     is a CM bug.]
>> >>>  2. CM to never check for null? Then we delete class "NullArgumentException".
>> >>>     Users are warned by the general policy: "Do not pass null unless it is
>> >>>     explicitly documented to be allowed." A bug will lead to the JVM raising
>> >>>     a NPE.
>> >>>  3. CM to sometimes check for null? Then "NullArgumentException" should
>> >>>     inherit from "NullPointerException" because the user will sometimes see
>> >>>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
>> >>>     not check) and both should thus belong to the same hierarchy (from the
>> >>>     user's point-of-view, there is no reason to handle them in different
>> >>>     ways since it's the exact same bug).
>> >>>     For the user, the consequence will be similar to alternative 2, with
>> >>>     sometimes more information about the failure and sometimes (marginally)
>> >>>     earlier detection.
>> >>
>> >> As stated above, my preference is
>> >>
>> >> 4.  CM APIs *may* disallow nulls explicitly.  When that is done, the
>> >> implementations *should* check parameters (as they *should* check
>> >> all other stated preconditions) and when preconditions are violated,
>> >> a MathIllegalArgumentException is thrown.  I don't care whether or
>> >> not we keep NAE.  If we drop it, we should make sure whatever
>> >> exception messages we used to use to indicate illegal null arguments
>> >> are still there.
>> >>
>> >> Phil
>> >>
>> > I like this option better than 3. So, I'll change my "vote" to option
>> > #2, and possibly option #4 as we will never agree on option #2.
>
> Why is it better to throw MathIllegalArgumentException rather than NPE?
>
Sorry, I misread Phil's proposal: I like the fact that nulls are
checked only if stated in the doc.
But I also think that the exception thrown should inherit from NPE, not IAE.
Your previous messages convinced me, but you have to admit that you
all wrote a lot on this issue, so it's difficult to keep up.
Nevertheless, I should have read more carefully.
Sébastien
>>
>> My preferences are 4 or 3 with same preference level and 2 as a fallback
>> choice. For each option, I find the arguments are good, it's only a
>> matter or preference.
>
> Did everyone consider that Phil's alternative implies a behaviour that
> departs from the standard practice, a.o.
>  * JDK (cf. quote in a previous post),
>  * other libraries, e.g. Guava (cf. quote in a previous post), and
>  * Java experts, e.g. Joshua Bloch[1]
> of throwing NPE when they encounter "null"?
> Doesn't anyone care about having a _reason_ for CM to behave differently?
>
> Don't you care that users will see different exceptions when the same
> problem is detected?
>
> Do you really like a policy that mandates different behaviours when "null"
> is disallowed implicitly vs explicitly (throwing NPE vs throwing
> MathRuntimeException)?  [Pardon me, but IMHO this is nonsense.]
>
> After piling up arguments (_none_ of which have been addressed), I'm sorry
> to read that it would only be a "matter of preference"! [I should start a
> career as a writer if I'm able to express "preference" in so many words...]
> Actually, it's a matter of consistency ("same problem, same exception").
> If nobody cares about improving CM's consistency, then indeed most of the
> discussions in which I take part are pointless.
>
> What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows
> CM to fail as early as possible (which I thought was Phil's main point, as
> it was his only one, apart from "preference"), while being consistent ("same
> problem, same exception"), internally, externally, implicitly and
> explicitly. Alternative 4 is inconsistent ("same problem, different
> exceptions"); it's a truth, not a matter of taste.]
>
> Rather then being willingly stuck in a deadlock because only the weakest
> argument ("preference") is taken into account, wouldn't it be more
> reasonable to count all the arguments?
>
>
> Gilles
>
> [1] "Effective Java" (second edition). Excerpt from "Item 60":
>     If a caller passes null in some parameter for which null values are
>     prohibited, convention dictates that NullPointerException be thrown
>     rather than IllegalArgumentException.
>
>>
>> Luc
>>
>> >
>> > Best regards,
>> > Sébastien
>> >>>
>> >>> Your alternative (sometimes check, sometimes not) is not fully consistent:
>> >>>  * for the user: "In case of null reference, will I get a MathRuntimeException
>> >>>    or a NPE?"
>> >>>  * for the CM developer: "In which cases do I need to check for null?"
>> >>>
>> >>> Of course, I would reconsider if you could provide an actual example that
>> >>> would fail with all three alternatives which I suggested. If not, then
>> >>> it seems obvious that your alternative presents two defects that don't exist
>> >>> in any of those three.
>> >>>
>> >>>
>> >>> Gilles
>> >>>
>> >>>>>> [...] what is different about null arguments at the point of
>> >>>>>> method activation in an API that explicitly disallows them.
>> >>>>> The difference is that there is no need to tell the user what the problem
>> >>>>> is because the raised exception says it all: "You tried to use a null
>> >>>>> reference." [As said above, the only issue is _when_ the exception is
>> >>>>> triggered.]
>> >>>>>
>> >>>>> The policy mandates what is globally valid, e.g.:
>> >>>>>   "If not specified otherwise, "null" is not allowed as an argument."
>> >>>>> Then, a user cannot complain about a NPE being propagated when he passed an
>> >>>>> invalid (null) argument.
>> >>>>>
>> >>>>> Finally, the case for "null" is also slightly peculiar (given the above)
>> >>>>> that it should not simply be bundled with the rationale "Fail early": Indeed
>> >>>>> "null" references always fail early (i.e. as soon as they are used).
>> >>>>> Deferring the check until it is done by the JVM will never entails the code
>> >>>>> to produce a wrong result _because_ of that null reference (it will just
>> >>>>> fail in the predictable way: NPE).[1]
>> >>>>>
>> >>>>>
>> >>>>> Gilles
>> >>>>>
>> >>>>> [1] Unlike in C, where an unitialized pointer would not necessarily crash
>> >>>>>     the program, but _could_ make it behave erratically (including produce
>> >>>>>     wrong results in a stealth way).
>> >>>>>
>> >>> ---------------------------------------------------------------------
>> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >>> For additional commands, e-mail: dev-help@commons.apache.org
>> >>>
>> >>>
>> >>
>> >>
>> >> ---------------------------------------------------------------------
>> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> >> For additional commands, e-mail: dev-help@commons.apache.org
>> >>
>> >
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > For additional commands, e-mail: dev-help@commons.apache.org
>> >
>> >
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/14/12 4:28 AM, Gilles Sadowski wrote:
> On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote:
>> Le 14/09/2012 08:46, Sébastien Brisard a écrit :
>>> Hi Phil
>>>
>>>>> Back to square one, with 3 fully consistent alternatives:
>>>>>  1. CM to always check for null? Then "NullArgumentException" inheriting from
>>>>>     "MathIllegalArgumentException" is fine because we promise to the user that
>>>>>     no NPE will ever propagate through the CM layer. [Breaking that promise
>>>>>     is a CM bug.]
>>>>>  2. CM to never check for null? Then we delete class "NullArgumentException".
>>>>>     Users are warned by the general policy: "Do not pass null unless it is
>>>>>     explicitly documented to be allowed." A bug will lead to the JVM raising
>>>>>     a NPE.
>>>>>  3. CM to sometimes check for null? Then "NullArgumentException" should
>>>>>     inherit from "NullPointerException" because the user will sometimes see
>>>>>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
>>>>>     not check) and both should thus belong to the same hierarchy (from the
>>>>>     user's point-of-view, there is no reason to handle them in different
>>>>>     ways since it's the exact same bug).
>>>>>     For the user, the consequence will be similar to alternative 2, with
>>>>>     sometimes more information about the failure and sometimes (marginally)
>>>>>     earlier detection.
>>>> As stated above, my preference is
>>>>
>>>> 4.  CM APIs *may* disallow nulls explicitly.  When that is done, the
>>>> implementations *should* check parameters (as they *should* check
>>>> all other stated preconditions) and when preconditions are violated,
>>>> a MathIllegalArgumentException is thrown.  I don't care whether or
>>>> not we keep NAE.  If we drop it, we should make sure whatever
>>>> exception messages we used to use to indicate illegal null arguments
>>>> are still there.
>>>>
>>>> Phil
>>>>
>>> I like this option better than 3. So, I'll change my "vote" to option
>>> #2, and possibly option #4 as we will never agree on option #2.
> Why is it better to throw MathIllegalArgumentException rather than NPE?

Because, as I have repeated numerous times, that is the appropriate
exception to throw when API preconditions are violated.  That is why
IAE exists.  The class javadoc comment for IAE is nice and succinct
(we could learn from it ;)   You could ask the same question about
ArrayIndexOutOfBounds.  Why not throw that instead of IAE when bad
indices are provided?   The difference is that instead of letting
the precondition failure go undetected and the runtime exception
propagate, APIs with explicit preconditions and parameter checking
in place raise the exception at method activation time.  In that
context, what is appropriate is IAE.

Phil
>
>> My preferences are 4 or 3 with same preference level and 2 as a fallback
>> choice. For each option, I find the arguments are good, it's only a
>> matter or preference.
> Did everyone consider that Phil's alternative implies a behaviour that
> departs from the standard practice, a.o. 
>  * JDK (cf. quote in a previous post),
>  * other libraries, e.g. Guava (cf. quote in a previous post), and
>  * Java experts, e.g. Joshua Bloch[1]
> of throwing NPE when they encounter "null"?
> Doesn't anyone care about having a _reason_ for CM to behave differently?
>
> Don't you care that users will see different exceptions when the same
> problem is detected?
>
> Do you really like a policy that mandates different behaviours when "null"
> is disallowed implicitly vs explicitly (throwing NPE vs throwing
> MathRuntimeException)?  [Pardon me, but IMHO this is nonsense.]
>
> After piling up arguments (_none_ of which have been addressed), I'm sorry
> to read that it would only be a "matter of preference"! [I should start a
> career as a writer if I'm able to express "preference" in so many words...]
> Actually, it's a matter of consistency ("same problem, same exception").
> If nobody cares about improving CM's consistency, then indeed most of the
> discussions in which I take part are pointless.
>
> What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows
> CM to fail as early as possible (which I thought was Phil's main point, as
> it was his only one, apart from "preference"), while being consistent ("same
> problem, same exception"), internally, externally, implicitly and
> explicitly. Alternative 4 is inconsistent ("same problem, different
> exceptions"); it's a truth, not a matter of taste.]
>
> Rather then being willingly stuck in a deadlock because only the weakest
> argument ("preference") is taken into account, wouldn't it be more
> reasonable to count all the arguments?
>
>
> Gilles
>
> [1] "Effective Java" (second edition). Excerpt from "Item 60":
>     If a caller passes null in some parameter for which null values are
>     prohibited, convention dictates that NullPointerException be thrown
>     rather than IllegalArgumentException.
>
>> Luc
>>
>>> Best regards,
>>> Sébastien
>>>>> Your alternative (sometimes check, sometimes not) is not fully consistent:
>>>>>  * for the user: "In case of null reference, will I get a MathRuntimeException
>>>>>    or a NPE?"
>>>>>  * for the CM developer: "In which cases do I need to check for null?"
>>>>>
>>>>> Of course, I would reconsider if you could provide an actual example that
>>>>> would fail with all three alternatives which I suggested. If not, then
>>>>> it seems obvious that your alternative presents two defects that don't exist
>>>>> in any of those three.
>>>>>
>>>>>
>>>>> Gilles
>>>>>
>>>>>>>> [...] what is different about null arguments at the point of
>>>>>>>> method activation in an API that explicitly disallows them.
>>>>>>> The difference is that there is no need to tell the user what the problem
>>>>>>> is because the raised exception says it all: "You tried to use a null
>>>>>>> reference." [As said above, the only issue is _when_ the exception is
>>>>>>> triggered.]
>>>>>>>
>>>>>>> The policy mandates what is globally valid, e.g.:
>>>>>>>   "If not specified otherwise, "null" is not allowed as an argument."
>>>>>>> Then, a user cannot complain about a NPE being propagated when he passed an
>>>>>>> invalid (null) argument.
>>>>>>>
>>>>>>> Finally, the case for "null" is also slightly peculiar (given the above)
>>>>>>> that it should not simply be bundled with the rationale "Fail early": Indeed
>>>>>>> "null" references always fail early (i.e. as soon as they are used).
>>>>>>> Deferring the check until it is done by the JVM will never entails the code
>>>>>>> to produce a wrong result _because_ of that null reference (it will just
>>>>>>> fail in the predictable way: NPE).[1]
>>>>>>>
>>>>>>>
>>>>>>> Gilles
>>>>>>>
>>>>>>> [1] Unlike in C, where an unitialized pointer would not necessarily crash
>>>>>>>     the program, but _could_ make it behave erratically (including produce
>>>>>>>     wrong results in a stealth way).
>>>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote:
> Le 14/09/2012 08:46, Sébastien Brisard a écrit :
> > Hi Phil
> > 
> >>>
> >>> Back to square one, with 3 fully consistent alternatives:
> >>>  1. CM to always check for null? Then "NullArgumentException" inheriting from
> >>>     "MathIllegalArgumentException" is fine because we promise to the user that
> >>>     no NPE will ever propagate through the CM layer. [Breaking that promise
> >>>     is a CM bug.]
> >>>  2. CM to never check for null? Then we delete class "NullArgumentException".
> >>>     Users are warned by the general policy: "Do not pass null unless it is
> >>>     explicitly documented to be allowed." A bug will lead to the JVM raising
> >>>     a NPE.
> >>>  3. CM to sometimes check for null? Then "NullArgumentException" should
> >>>     inherit from "NullPointerException" because the user will sometimes see
> >>>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
> >>>     not check) and both should thus belong to the same hierarchy (from the
> >>>     user's point-of-view, there is no reason to handle them in different
> >>>     ways since it's the exact same bug).
> >>>     For the user, the consequence will be similar to alternative 2, with
> >>>     sometimes more information about the failure and sometimes (marginally)
> >>>     earlier detection.
> >>
> >> As stated above, my preference is
> >>
> >> 4.  CM APIs *may* disallow nulls explicitly.  When that is done, the
> >> implementations *should* check parameters (as they *should* check
> >> all other stated preconditions) and when preconditions are violated,
> >> a MathIllegalArgumentException is thrown.  I don't care whether or
> >> not we keep NAE.  If we drop it, we should make sure whatever
> >> exception messages we used to use to indicate illegal null arguments
> >> are still there.
> >>
> >> Phil
> >>
> > I like this option better than 3. So, I'll change my "vote" to option
> > #2, and possibly option #4 as we will never agree on option #2.

Why is it better to throw MathIllegalArgumentException rather than NPE?

> 
> My preferences are 4 or 3 with same preference level and 2 as a fallback
> choice. For each option, I find the arguments are good, it's only a
> matter or preference.

Did everyone consider that Phil's alternative implies a behaviour that
departs from the standard practice, a.o. 
 * JDK (cf. quote in a previous post),
 * other libraries, e.g. Guava (cf. quote in a previous post), and
 * Java experts, e.g. Joshua Bloch[1]
of throwing NPE when they encounter "null"?
Doesn't anyone care about having a _reason_ for CM to behave differently?

Don't you care that users will see different exceptions when the same
problem is detected?

Do you really like a policy that mandates different behaviours when "null"
is disallowed implicitly vs explicitly (throwing NPE vs throwing
MathRuntimeException)?  [Pardon me, but IMHO this is nonsense.]

After piling up arguments (_none_ of which have been addressed), I'm sorry
to read that it would only be a "matter of preference"! [I should start a
career as a writer if I'm able to express "preference" in so many words...]
Actually, it's a matter of consistency ("same problem, same exception").
If nobody cares about improving CM's consistency, then indeed most of the
discussions in which I take part are pointless.

What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows
CM to fail as early as possible (which I thought was Phil's main point, as
it was his only one, apart from "preference"), while being consistent ("same
problem, same exception"), internally, externally, implicitly and
explicitly. Alternative 4 is inconsistent ("same problem, different
exceptions"); it's a truth, not a matter of taste.]

Rather then being willingly stuck in a deadlock because only the weakest
argument ("preference") is taken into account, wouldn't it be more
reasonable to count all the arguments?


Gilles

[1] "Effective Java" (second edition). Excerpt from "Item 60":
    If a caller passes null in some parameter for which null values are
    prohibited, convention dictates that NullPointerException be thrown
    rather than IllegalArgumentException.

> 
> Luc
> 
> > 
> > Best regards,
> > Sébastien
> >>>
> >>> Your alternative (sometimes check, sometimes not) is not fully consistent:
> >>>  * for the user: "In case of null reference, will I get a MathRuntimeException
> >>>    or a NPE?"
> >>>  * for the CM developer: "In which cases do I need to check for null?"
> >>>
> >>> Of course, I would reconsider if you could provide an actual example that
> >>> would fail with all three alternatives which I suggested. If not, then
> >>> it seems obvious that your alternative presents two defects that don't exist
> >>> in any of those three.
> >>>
> >>>
> >>> Gilles
> >>>
> >>>>>> [...] what is different about null arguments at the point of
> >>>>>> method activation in an API that explicitly disallows them.
> >>>>> The difference is that there is no need to tell the user what the problem
> >>>>> is because the raised exception says it all: "You tried to use a null
> >>>>> reference." [As said above, the only issue is _when_ the exception is
> >>>>> triggered.]
> >>>>>
> >>>>> The policy mandates what is globally valid, e.g.:
> >>>>>   "If not specified otherwise, "null" is not allowed as an argument."
> >>>>> Then, a user cannot complain about a NPE being propagated when he passed an
> >>>>> invalid (null) argument.
> >>>>>
> >>>>> Finally, the case for "null" is also slightly peculiar (given the above)
> >>>>> that it should not simply be bundled with the rationale "Fail early": Indeed
> >>>>> "null" references always fail early (i.e. as soon as they are used).
> >>>>> Deferring the check until it is done by the JVM will never entails the code
> >>>>> to produce a wrong result _because_ of that null reference (it will just
> >>>>> fail in the predictable way: NPE).[1]
> >>>>>
> >>>>>
> >>>>> Gilles
> >>>>>
> >>>>> [1] Unlike in C, where an unitialized pointer would not necessarily crash
> >>>>>     the program, but _could_ make it behave erratically (including produce
> >>>>>     wrong results in a stealth way).
> >>>>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>
> >>>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >>
> > 
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> > 
> > 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 14/09/2012 08:46, Sébastien Brisard a écrit :
> Hi Phil
> 
>>>
>>> Back to square one, with 3 fully consistent alternatives:
>>>  1. CM to always check for null? Then "NullArgumentException" inheriting from
>>>     "MathIllegalArgumentException" is fine because we promise to the user that
>>>     no NPE will ever propagate through the CM layer. [Breaking that promise
>>>     is a CM bug.]
>>>  2. CM to never check for null? Then we delete class "NullArgumentException".
>>>     Users are warned by the general policy: "Do not pass null unless it is
>>>     explicitly documented to be allowed." A bug will lead to the JVM raising
>>>     a NPE.
>>>  3. CM to sometimes check for null? Then "NullArgumentException" should
>>>     inherit from "NullPointerException" because the user will sometimes see
>>>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
>>>     not check) and both should thus belong to the same hierarchy (from the
>>>     user's point-of-view, there is no reason to handle them in different
>>>     ways since it's the exact same bug).
>>>     For the user, the consequence will be similar to alternative 2, with
>>>     sometimes more information about the failure and sometimes (marginally)
>>>     earlier detection.
>>
>> As stated above, my preference is
>>
>> 4.  CM APIs *may* disallow nulls explicitly.  When that is done, the
>> implementations *should* check parameters (as they *should* check
>> all other stated preconditions) and when preconditions are violated,
>> a MathIllegalArgumentException is thrown.  I don't care whether or
>> not we keep NAE.  If we drop it, we should make sure whatever
>> exception messages we used to use to indicate illegal null arguments
>> are still there.
>>
>> Phil
>>
> I like this option better than 3. So, I'll change my "vote" to option
> #2, and possibly option #4 as we will never agree on option #2.

My preferences are 4 or 3 with same preference level and 2 as a fallback
choice. For each option, I find the arguments are good, it's only a
matter or preference.

Luc

> 
> Best regards,
> Sébastien
>>>
>>> Your alternative (sometimes check, sometimes not) is not fully consistent:
>>>  * for the user: "In case of null reference, will I get a MathRuntimeException
>>>    or a NPE?"
>>>  * for the CM developer: "In which cases do I need to check for null?"
>>>
>>> Of course, I would reconsider if you could provide an actual example that
>>> would fail with all three alternatives which I suggested. If not, then
>>> it seems obvious that your alternative presents two defects that don't exist
>>> in any of those three.
>>>
>>>
>>> Gilles
>>>
>>>>>> [...] what is different about null arguments at the point of
>>>>>> method activation in an API that explicitly disallows them.
>>>>> The difference is that there is no need to tell the user what the problem
>>>>> is because the raised exception says it all: "You tried to use a null
>>>>> reference." [As said above, the only issue is _when_ the exception is
>>>>> triggered.]
>>>>>
>>>>> The policy mandates what is globally valid, e.g.:
>>>>>   "If not specified otherwise, "null" is not allowed as an argument."
>>>>> Then, a user cannot complain about a NPE being propagated when he passed an
>>>>> invalid (null) argument.
>>>>>
>>>>> Finally, the case for "null" is also slightly peculiar (given the above)
>>>>> that it should not simply be bundled with the rationale "Fail early": Indeed
>>>>> "null" references always fail early (i.e. as soon as they are used).
>>>>> Deferring the check until it is done by the JVM will never entails the code
>>>>> to produce a wrong result _because_ of that null reference (it will just
>>>>> fail in the predictable way: NPE).[1]
>>>>>
>>>>>
>>>>> Gilles
>>>>>
>>>>> [1] Unlike in C, where an unitialized pointer would not necessarily crash
>>>>>     the program, but _could_ make it behave erratically (including produce
>>>>>     wrong results in a stealth way).
>>>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hi Phil

>>
>> Back to square one, with 3 fully consistent alternatives:
>>  1. CM to always check for null? Then "NullArgumentException" inheriting from
>>     "MathIllegalArgumentException" is fine because we promise to the user that
>>     no NPE will ever propagate through the CM layer. [Breaking that promise
>>     is a CM bug.]
>>  2. CM to never check for null? Then we delete class "NullArgumentException".
>>     Users are warned by the general policy: "Do not pass null unless it is
>>     explicitly documented to be allowed." A bug will lead to the JVM raising
>>     a NPE.
>>  3. CM to sometimes check for null? Then "NullArgumentException" should
>>     inherit from "NullPointerException" because the user will sometimes see
>>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
>>     not check) and both should thus belong to the same hierarchy (from the
>>     user's point-of-view, there is no reason to handle them in different
>>     ways since it's the exact same bug).
>>     For the user, the consequence will be similar to alternative 2, with
>>     sometimes more information about the failure and sometimes (marginally)
>>     earlier detection.
>
> As stated above, my preference is
>
> 4.  CM APIs *may* disallow nulls explicitly.  When that is done, the
> implementations *should* check parameters (as they *should* check
> all other stated preconditions) and when preconditions are violated,
> a MathIllegalArgumentException is thrown.  I don't care whether or
> not we keep NAE.  If we drop it, we should make sure whatever
> exception messages we used to use to indicate illegal null arguments
> are still there.
>
> Phil
>
I like this option better than 3. So, I'll change my "vote" to option
#2, and possibly option #4 as we will never agree on option #2.

Best regards,
Sébastien
>>
>> Your alternative (sometimes check, sometimes not) is not fully consistent:
>>  * for the user: "In case of null reference, will I get a MathRuntimeException
>>    or a NPE?"
>>  * for the CM developer: "In which cases do I need to check for null?"
>>
>> Of course, I would reconsider if you could provide an actual example that
>> would fail with all three alternatives which I suggested. If not, then
>> it seems obvious that your alternative presents two defects that don't exist
>> in any of those three.
>>
>>
>> Gilles
>>
>>>>> [...] what is different about null arguments at the point of
>>>>> method activation in an API that explicitly disallows them.
>>>> The difference is that there is no need to tell the user what the problem
>>>> is because the raised exception says it all: "You tried to use a null
>>>> reference." [As said above, the only issue is _when_ the exception is
>>>> triggered.]
>>>>
>>>> The policy mandates what is globally valid, e.g.:
>>>>   "If not specified otherwise, "null" is not allowed as an argument."
>>>> Then, a user cannot complain about a NPE being propagated when he passed an
>>>> invalid (null) argument.
>>>>
>>>> Finally, the case for "null" is also slightly peculiar (given the above)
>>>> that it should not simply be bundled with the rationale "Fail early": Indeed
>>>> "null" references always fail early (i.e. as soon as they are used).
>>>> Deferring the check until it is done by the JVM will never entails the code
>>>> to produce a wrong result _because_ of that null reference (it will just
>>>> fail in the predictable way: NPE).[1]
>>>>
>>>>
>>>> Gilles
>>>>
>>>> [1] Unlike in C, where an unitialized pointer would not necessarily crash
>>>>     the program, but _could_ make it behave erratically (including produce
>>>>     wrong results in a stealth way).
>>>>
>> ---------------------------------------------------------------------
>> 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] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/12/12 4:10 PM, Gilles Sadowski wrote:
> Hello.
>
> [For those who don't wish to read the whole post, please at least go towards
> the end and indicate your preferred option. Thanks.]
>
>>> [...]
>>>> All are bad arguments violating API contract,
>>>> all detected at method activation time -> IAE.
>>> Agreed... if the policy is to _always_ check for "null"!
>> I am only referring to APIs that explicitly disallow nulls.
> I don't know what you mean by "only": Almost _all_ the API disallows "null".
> What has "implicitly" or "explicitly" have to do with that (other than
> "implicitly" being equivalent to lacking documentation vs "explicitly" being
> defined by a default policy)?
>
>>  I have
>> agreed that we will not always do this.
> Again, this is one point where we differ as regards to "rules". I contend
> that there must be an "ideal", defined by the policy. Is the ideal to always
> check or to never check. I think the latter, for the reasons explained.
>
> The policy cannot be "sometimes yes, sometimes no" unless you can define in
> which case to do it and in which not, which is clearly more complicated.
>
>> The specific case being
>> discussed here is an API that is going to include as an explicit
>> precondition that nulls are not allowed.  When this is done, what
>> should be thrown is an IllegalArgumentException, which for us means
>> some subclass of MathIllegalArgumentException.
> If we were to decide that the ideal is to always check (an obviously
> different policy), I wouldn't have a problem with this.
> What I point at is the potential burden on the _user_ if he wants to
> intercept exceptions: for the _same_ problem (null reference), they'd have
> to catch two types of exceptions: NPE vs NAE (as sublcass of MIAE).
>
> If nobody cares about that, I won't either. I just wanted to stress that we
> might have some user complaints about this inconsistency which will appear
> at the contact point between user code and CM code: Because it is simply not
> nice to report the same problem in different ways.
>
> And because it is inconsistent, some future contributor will raise this
> issue again: "Why do we check here and throw MIAE and why don't we check
> there and let the JVM throw NPE?". [This is exactly what you reported about
> checking the components of a FieldVector but not the vector itself.]
>
>>> If we sometimes check and sometimes not, the thrown exception will be from a
>>> different hierarchy (NPE vs MathRuntimeException). This is annoying (and
>>> inconsistent).
>> That is an unfortunate consequence of not always checking.  I
>> understand that it may be impractical to always check.  Therefore,
>> some NPEs are going to be propagated to clients with no warning. 
> What do you mean by "impractical"?
> In practice, we will not check everything tomorrow, but we can decide to
> work toward that goal.
> But we have to answer the question not of whether it is practical, but
> whether it is desirable.
> This is a quite simple issue; and I don't believe in considerations like
> it might be good in some part of CM APIs but not in others. [In _principle_
> I mean; I don't say that in _practice_ some parts of CM won't be up-to-date
> sooner than other parts.]
>
>>> The oft-repeated issue is "Is it useful to check for null or not?"; at least
>>> 4 people answered "No", because this bug will never go unnoticed: sooner or
>>> later, using "null" _will_ raise exception. The sole and unique difference
>>> with CM checking and JVM checking is that the stack trace will (sometimes)
>>> be a little shorter in the former case.
>> And inferior first failure data capture and potential additional
>> computation, side effects or other unknown consequences (unless we
>> always think through the consequences of nulls propagated through
>> our code and make sure there are never any untoward side effects. 
>> All of these are reasons for the "when you must fail, fail fast and
>> loudly" maxim.  I understand that it may not be possible to adhere
>> to this fully in [math].
> You repeat the mantra "fail fast, etc." but do not give a counter-argument
> to mine about the NPE being as fast as it needs to be for this kind of
> failure.
> I'd like to see one example of "untoward side effects".
>
>>>  Most people have come to agree that
>>> it's not worth adopting a policy of thorough checking. [Rationale 1: users
>>> who encounter a NPE will need to inspect the stack trace and go to _their_
>>> code in order to see where _they_ put a "null". Rationale 2: there is
>>> probably a performance penalty to all these checks (especially in a well
>>> tested code where "null" reference bugs have been ironed out).]
>> The latter is a good point and why I am OK not checking everywhere.
> The argument is equally valid _everywhere_ (unless you can provide the
> above example), in both directions: either it is good to check, then it is
> always good, or in some cases it is not necessary, then it is never
> necessary.
>
>>>> Consider, e.g. an empty array or array indices that will lead to
>>>> index out of bounds errors in APIs that take arrays.  What is so
>>>> different about null?
>>> There is no difference... if there is a policy that decrees so.
>>>
>>> It is _not_ the policy of the JDK: "NullPointerException" is not the same
>>> error as "IndexOutOfBoundsException"; their common parent class is the
>>> (unspecific) "RuntimeException" and their sibling is
>>> "IllegalArgumentException".
>>> There is no "is-a" relationship between NPE, IOBE and IAE.
>> Here you are missing the point above - we are talking about specific
>> APIs that explicitly disallow nulls.
> I do not think I miss any point. Rather it is you who fail to point to
> to cases where such an "explicit" mention is needed.
> As Luc pointed out, only stating where null _is_ allowed is actually
> useful. And as I pointed out about your example of using null in
> constructors, those occurrences should be very rare, for the very reason
> that such code is much more obscure (and documentation should thus be
> extremely precise to avoid confusion).
>
>>  Just as we trap or check
>> arguments to avoid array index errors, we can in some cases do the
>> same for nulls.  In these cases, it is appropriate to throw IAE for
>> the nulls just as we do for the (avoided) AIOBE.
>>> Recalling that some 1.5 years ago you were against the adoption of the
>>> single exception hierarchy, rooted at "MathRuntimeException", in order to
>>> stay faithful to the JDK exception types, I wonder why you are on the
>>> opposite stand as NPE is concerned.
>> I am not.  I have agreed that we can go back to a common root.  But
>> that then forces us to create surrogates for the top level
>> exceptions such as IAE that the "math" versions used to subclass.  I
>> am OK with that.  But we should maintain the semantics correctly,
>> throwing a MIAE when API contracts are violated.
> You don't answer to my argument. I did not say that you were against a
> common root _now_, but that you preferred to retain the standard exceptions
> _before_. For the one exception where I think that it is better to indeed
> keep the standard NPE exception, you hold the opposite view (common root).
>
> In summary, you agree that it might not be good (or "practical", whatever
> that means) to always check for null. Then, where do you draw the line in
> order to take "Rationale 2" into account? IOW, you say that "Rationale 2"
> is a good point but, imagine that someone spends a few days adding all the
> null checks, then this good point will not exist anymore. That's a
> contradiction.
>
> Again, having NAE inherit from NPE removes all contradictions in one fell
> swoop. I can't fathom why you insist on the naming issue, why this
> precondition should be reported by "MathIllegalArgumentException". I repeat
> that in the JDK "ArrayIndexOutOfBoundsException" is _not_ a subclass of
> "IllegalArgumentException". That, in CM, "OutOfRangeException" _is_ a
> subclass of "MathIllegalArgumentException" is a different choice that _we_
> made. We can make a different choice for "NullArgumentException" if we see
> that there is more benefit: it allows a flexible, yet consistent policy, of
> checking or not checking while throwing the same exception for the same
> problem.
>
> Back to square one, with 3 fully consistent alternatives:
>  1. CM to always check for null? Then "NullArgumentException" inheriting from
>     "MathIllegalArgumentException" is fine because we promise to the user that
>     no NPE will ever propagate through the CM layer. [Breaking that promise
>     is a CM bug.]
>  2. CM to never check for null? Then we delete class "NullArgumentException".
>     Users are warned by the general policy: "Do not pass null unless it is
>     explicitly documented to be allowed." A bug will lead to the JVM raising
>     a NPE.
>  3. CM to sometimes check for null? Then "NullArgumentException" should
>     inherit from "NullPointerException" because the user will sometimes see
>     "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
>     not check) and both should thus belong to the same hierarchy (from the
>     user's point-of-view, there is no reason to handle them in different
>     ways since it's the exact same bug).
>     For the user, the consequence will be similar to alternative 2, with
>     sometimes more information about the failure and sometimes (marginally)
>     earlier detection.

As stated above, my preference is

4.  CM APIs *may* disallow nulls explicitly.  When that is done, the
implementations *should* check parameters (as they *should* check
all other stated preconditions) and when preconditions are violated,
a MathIllegalArgumentException is thrown.  I don't care whether or
not we keep NAE.  If we drop it, we should make sure whatever
exception messages we used to use to indicate illegal null arguments
are still there.

Phil
>
> Your alternative (sometimes check, sometimes not) is not fully consistent:
>  * for the user: "In case of null reference, will I get a MathRuntimeException
>    or a NPE?"
>  * for the CM developer: "In which cases do I need to check for null?"
>
> Of course, I would reconsider if you could provide an actual example that
> would fail with all three alternatives which I suggested. If not, then
> it seems obvious that your alternative presents two defects that don't exist
> in any of those three.
>
>
> Gilles
>
>>>> [...] what is different about null arguments at the point of
>>>> method activation in an API that explicitly disallows them.
>>> The difference is that there is no need to tell the user what the problem
>>> is because the raised exception says it all: "You tried to use a null
>>> reference." [As said above, the only issue is _when_ the exception is
>>> triggered.]
>>>
>>> The policy mandates what is globally valid, e.g.:
>>>   "If not specified otherwise, "null" is not allowed as an argument."
>>> Then, a user cannot complain about a NPE being propagated when he passed an
>>> invalid (null) argument.
>>>
>>> Finally, the case for "null" is also slightly peculiar (given the above)
>>> that it should not simply be bundled with the rationale "Fail early": Indeed
>>> "null" references always fail early (i.e. as soon as they are used).
>>> Deferring the check until it is done by the JVM will never entails the code
>>> to produce a wrong result _because_ of that null reference (it will just
>>> fail in the predictable way: NPE).[1]
>>>
>>>
>>> Gilles
>>>
>>> [1] Unlike in C, where an unitialized pointer would not necessarily crash
>>>     the program, but _could_ make it behave erratically (including produce
>>>     wrong results in a stealth way).
>>>
> ---------------------------------------------------------------------
> 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] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hello.

[For those who don't wish to read the whole post, please at least go towards
the end and indicate your preferred option. Thanks.]

> > [...]
> >> All are bad arguments violating API contract,
> >> all detected at method activation time -> IAE.
> > Agreed... if the policy is to _always_ check for "null"!
> 
> I am only referring to APIs that explicitly disallow nulls.

I don't know what you mean by "only": Almost _all_ the API disallows "null".
What has "implicitly" or "explicitly" have to do with that (other than
"implicitly" being equivalent to lacking documentation vs "explicitly" being
defined by a default policy)?

>  I have
> agreed that we will not always do this.

Again, this is one point where we differ as regards to "rules". I contend
that there must be an "ideal", defined by the policy. Is the ideal to always
check or to never check. I think the latter, for the reasons explained.

The policy cannot be "sometimes yes, sometimes no" unless you can define in
which case to do it and in which not, which is clearly more complicated.

> The specific case being
> discussed here is an API that is going to include as an explicit
> precondition that nulls are not allowed.  When this is done, what
> should be thrown is an IllegalArgumentException, which for us means
> some subclass of MathIllegalArgumentException.

If we were to decide that the ideal is to always check (an obviously
different policy), I wouldn't have a problem with this.
What I point at is the potential burden on the _user_ if he wants to
intercept exceptions: for the _same_ problem (null reference), they'd have
to catch two types of exceptions: NPE vs NAE (as sublcass of MIAE).

If nobody cares about that, I won't either. I just wanted to stress that we
might have some user complaints about this inconsistency which will appear
at the contact point between user code and CM code: Because it is simply not
nice to report the same problem in different ways.

And because it is inconsistent, some future contributor will raise this
issue again: "Why do we check here and throw MIAE and why don't we check
there and let the JVM throw NPE?". [This is exactly what you reported about
checking the components of a FieldVector but not the vector itself.]

> >
> > If we sometimes check and sometimes not, the thrown exception will be from a
> > different hierarchy (NPE vs MathRuntimeException). This is annoying (and
> > inconsistent).
> 
> That is an unfortunate consequence of not always checking.  I
> understand that it may be impractical to always check.  Therefore,
> some NPEs are going to be propagated to clients with no warning. 

What do you mean by "impractical"?
In practice, we will not check everything tomorrow, but we can decide to
work toward that goal.
But we have to answer the question not of whether it is practical, but
whether it is desirable.
This is a quite simple issue; and I don't believe in considerations like
it might be good in some part of CM APIs but not in others. [In _principle_
I mean; I don't say that in _practice_ some parts of CM won't be up-to-date
sooner than other parts.]

> >
> > The oft-repeated issue is "Is it useful to check for null or not?"; at least
> > 4 people answered "No", because this bug will never go unnoticed: sooner or
> > later, using "null" _will_ raise exception. The sole and unique difference
> > with CM checking and JVM checking is that the stack trace will (sometimes)
> > be a little shorter in the former case.
> 
> And inferior first failure data capture and potential additional
> computation, side effects or other unknown consequences (unless we
> always think through the consequences of nulls propagated through
> our code and make sure there are never any untoward side effects. 
> All of these are reasons for the "when you must fail, fail fast and
> loudly" maxim.  I understand that it may not be possible to adhere
> to this fully in [math].

You repeat the mantra "fail fast, etc." but do not give a counter-argument
to mine about the NPE being as fast as it needs to be for this kind of
failure.
I'd like to see one example of "untoward side effects".

> >  Most people have come to agree that
> > it's not worth adopting a policy of thorough checking. [Rationale 1: users
> > who encounter a NPE will need to inspect the stack trace and go to _their_
> > code in order to see where _they_ put a "null". Rationale 2: there is
> > probably a performance penalty to all these checks (especially in a well
> > tested code where "null" reference bugs have been ironed out).]
> 
> The latter is a good point and why I am OK not checking everywhere.

The argument is equally valid _everywhere_ (unless you can provide the
above example), in both directions: either it is good to check, then it is
always good, or in some cases it is not necessary, then it is never
necessary.

> >
> >> Consider, e.g. an empty array or array indices that will lead to
> >> index out of bounds errors in APIs that take arrays.  What is so
> >> different about null?
> > There is no difference... if there is a policy that decrees so.
> >
> > It is _not_ the policy of the JDK: "NullPointerException" is not the same
> > error as "IndexOutOfBoundsException"; their common parent class is the
> > (unspecific) "RuntimeException" and their sibling is
> > "IllegalArgumentException".
> > There is no "is-a" relationship between NPE, IOBE and IAE.
> 
> Here you are missing the point above - we are talking about specific
> APIs that explicitly disallow nulls.

I do not think I miss any point. Rather it is you who fail to point to
to cases where such an "explicit" mention is needed.
As Luc pointed out, only stating where null _is_ allowed is actually
useful. And as I pointed out about your example of using null in
constructors, those occurrences should be very rare, for the very reason
that such code is much more obscure (and documentation should thus be
extremely precise to avoid confusion).

>  Just as we trap or check
> arguments to avoid array index errors, we can in some cases do the
> same for nulls.  In these cases, it is appropriate to throw IAE for
> the nulls just as we do for the (avoided) AIOBE.
> >
> > Recalling that some 1.5 years ago you were against the adoption of the
> > single exception hierarchy, rooted at "MathRuntimeException", in order to
> > stay faithful to the JDK exception types, I wonder why you are on the
> > opposite stand as NPE is concerned.
> 
> I am not.  I have agreed that we can go back to a common root.  But
> that then forces us to create surrogates for the top level
> exceptions such as IAE that the "math" versions used to subclass.  I
> am OK with that.  But we should maintain the semantics correctly,
> throwing a MIAE when API contracts are violated.

You don't answer to my argument. I did not say that you were against a
common root _now_, but that you preferred to retain the standard exceptions
_before_. For the one exception where I think that it is better to indeed
keep the standard NPE exception, you hold the opposite view (common root).

In summary, you agree that it might not be good (or "practical", whatever
that means) to always check for null. Then, where do you draw the line in
order to take "Rationale 2" into account? IOW, you say that "Rationale 2"
is a good point but, imagine that someone spends a few days adding all the
null checks, then this good point will not exist anymore. That's a
contradiction.

Again, having NAE inherit from NPE removes all contradictions in one fell
swoop. I can't fathom why you insist on the naming issue, why this
precondition should be reported by "MathIllegalArgumentException". I repeat
that in the JDK "ArrayIndexOutOfBoundsException" is _not_ a subclass of
"IllegalArgumentException". That, in CM, "OutOfRangeException" _is_ a
subclass of "MathIllegalArgumentException" is a different choice that _we_
made. We can make a different choice for "NullArgumentException" if we see
that there is more benefit: it allows a flexible, yet consistent policy, of
checking or not checking while throwing the same exception for the same
problem.

Back to square one, with 3 fully consistent alternatives:
 1. CM to always check for null? Then "NullArgumentException" inheriting from
    "MathIllegalArgumentException" is fine because we promise to the user that
    no NPE will ever propagate through the CM layer. [Breaking that promise
    is a CM bug.]
 2. CM to never check for null? Then we delete class "NullArgumentException".
    Users are warned by the general policy: "Do not pass null unless it is
    explicitly documented to be allowed." A bug will lead to the JVM raising
    a NPE.
 3. CM to sometimes check for null? Then "NullArgumentException" should
    inherit from "NullPointerException" because the user will sometimes see
    "NullArgumentException" (when CM checks) and sometimes NPE (when CM does
    not check) and both should thus belong to the same hierarchy (from the
    user's point-of-view, there is no reason to handle them in different
    ways since it's the exact same bug).
    For the user, the consequence will be similar to alternative 2, with
    sometimes more information about the failure and sometimes (marginally)
    earlier detection.

Your alternative (sometimes check, sometimes not) is not fully consistent:
 * for the user: "In case of null reference, will I get a MathRuntimeException
   or a NPE?"
 * for the CM developer: "In which cases do I need to check for null?"

Of course, I would reconsider if you could provide an actual example that
would fail with all three alternatives which I suggested. If not, then
it seems obvious that your alternative presents two defects that don't exist
in any of those three.


Gilles

> >
> >> [...] what is different about null arguments at the point of
> >> method activation in an API that explicitly disallows them.
> > The difference is that there is no need to tell the user what the problem
> > is because the raised exception says it all: "You tried to use a null
> > reference." [As said above, the only issue is _when_ the exception is
> > triggered.]
> >
> > The policy mandates what is globally valid, e.g.:
> >   "If not specified otherwise, "null" is not allowed as an argument."
> > Then, a user cannot complain about a NPE being propagated when he passed an
> > invalid (null) argument.
> >
> > Finally, the case for "null" is also slightly peculiar (given the above)
> > that it should not simply be bundled with the rationale "Fail early": Indeed
> > "null" references always fail early (i.e. as soon as they are used).
> > Deferring the check until it is done by the JVM will never entails the code
> > to produce a wrong result _because_ of that null reference (it will just
> > fail in the predictable way: NPE).[1]
> >
> >
> > Gilles
> >
> > [1] Unlike in C, where an unitialized pointer would not necessarily crash
> >     the program, but _could_ make it behave erratically (including produce
> >     wrong results in a stealth way).
> >

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/12/12 8:52 AM, Gilles Sadowski wrote:
>> [...]
>>
>> I know this is a little of a sore subject, but in this and other
>> cases where arguments violate documented preconditions, I think we
>> should throw IAE, which means MNAE is only appropriate as long as it
>> continues to subclass our surrogate for IAE - MIAE.
>>
>> If I sound hard-headed here, it would be great if someone could
>> explain to me what is different about null arguments at the point of
>> method activation in an API that explicitly disallows them. 
>> Consider, e.g. an empty array or array indices that will lead to
>> index out of bounds errors in APIs that take arrays.  What is so
>> different about null?  All are bad arguments violating API contract,
>> all detected at method activation time -> IAE.
> Okay, I'm going to lay out again the arguments (all which I know about were
> already mentioned in the thread).
> [I'll try to demonstrate that where we differ is on the usefulness of having
> a policy!]
>
> Taking quotes from your mail in reversed order:
>
>> All are bad arguments violating API contract,
>> all detected at method activation time -> IAE.
> Agreed... if the policy is to _always_ check for "null"!

I am only referring to APIs that explicitly disallow nulls.  I have
agreed that we will not always do this.  The specific case being
discussed here is an API that is going to include as an explicit
precondition that nulls are not allowed.  When this is done, what
should be thrown is an IllegalArgumentException, which for us means
some subclass of MathIllegalArgumentException.
>
> If we sometimes check and sometimes not, the thrown exception will be from a
> different hierarchy (NPE vs MathRuntimeException). This is annoying (and
> inconsistent).

That is an unfortunate consequence of not always checking.  I
understand that it may be impractical to always check.  Therefore,
some NPEs are going to be propagated to clients with no warning. 
>
> The oft-repeated issue is "Is it useful to check for null or not?"; at least
> 4 people answered "No", because this bug will never go unnoticed: sooner or
> later, using "null" _will_ raise exception. The sole and unique difference
> with CM checking and JVM checking is that the stack trace will (sometimes)
> be a little shorter in the former case.

And inferior first failure data capture and potential additional
computation, side effects or other unknown consequences (unless we
always think through the consequences of nulls propagated through
our code and make sure there are never any untoward side effects. 
All of these are reasons for the "when you must fail, fail fast and
loudly" maxim.  I understand that it may not be possible to adhere
to this fully in [math].

>  Most people have come to agree that
> it's not worth adopting a policy of thorough checking. [Rationale 1: users
> who encounter a NPE will need to inspect the stack trace and go to _their_
> code in order to see where _they_ put a "null". Rationale 2: there is
> probably a performance penalty to all these checks (especially in a well
> tested code where "null" reference bugs have been ironed out).]

The latter is a good point and why I am OK not checking everywhere.
>
>> Consider, e.g. an empty array or array indices that will lead to
>> index out of bounds errors in APIs that take arrays.  What is so
>> different about null?
> There is no difference... if there is a policy that decrees so.
>
> It is _not_ the policy of the JDK: "NullPointerException" is not the same
> error as "IndexOutOfBoundsException"; their common parent class is the
> (unspecific) "RuntimeException" and their sibling is
> "IllegalArgumentException".
> There is no "is-a" relationship between NPE, IOBE and IAE.

Here you are missing the point above - we are talking about specific
APIs that explicitly disallow nulls.  Just as we trap or check
arguments to avoid array index errors, we can in some cases do the
same for nulls.  In these cases, it is appropriate to throw IAE for
the nulls just as we do for the (avoided) AIOBE.
>
> Recalling that some 1.5 years ago you were against the adoption of the
> single exception hierarchy, rooted at "MathRuntimeException", in order to
> stay faithful to the JDK exception types, I wonder why you are on the
> opposite stand as NPE is concerned.

I am not.  I have agreed that we can go back to a common root.  But
that then forces us to create surrogates for the top level
exceptions such as IAE that the "math" versions used to subclass.  I
am OK with that.  But we should maintain the semantics correctly,
throwing a MIAE when API contracts are violated.
>
>> [...] what is different about null arguments at the point of
>> method activation in an API that explicitly disallows them.
> The difference is that there is no need to tell the user what the problem
> is because the raised exception says it all: "You tried to use a null
> reference." [As said above, the only issue is _when_ the exception is
> triggered.]
>
> The policy mandates what is globally valid, e.g.:
>   "If not specified otherwise, "null" is not allowed as an argument."
> Then, a user cannot complain about a NPE being propagated when he passed an
> invalid (null) argument.
>
> Finally, the case for "null" is also slightly peculiar (given the above)
> that it should not simply be bundled with the rationale "Fail early": Indeed
> "null" references always fail early (i.e. as soon as they are used).
> Deferring the check until it is done by the JVM will never entails the code
> to produce a wrong result _because_ of that null reference (it will just
> fail in the predictable way: NPE).[1]
>
>
> Gilles
>
> [1] Unlike in C, where an unitialized pointer would not necessarily crash
>     the program, but _could_ make it behave erratically (including produce
>     wrong results in a stealth way).
>
> ---------------------------------------------------------------------
> 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] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> [...]
> 
> I know this is a little of a sore subject, but in this and other
> cases where arguments violate documented preconditions, I think we
> should throw IAE, which means MNAE is only appropriate as long as it
> continues to subclass our surrogate for IAE - MIAE.
> 
> If I sound hard-headed here, it would be great if someone could
> explain to me what is different about null arguments at the point of
> method activation in an API that explicitly disallows them. 
> Consider, e.g. an empty array or array indices that will lead to
> index out of bounds errors in APIs that take arrays.  What is so
> different about null?  All are bad arguments violating API contract,
> all detected at method activation time -> IAE.

Okay, I'm going to lay out again the arguments (all which I know about were
already mentioned in the thread).
[I'll try to demonstrate that where we differ is on the usefulness of having
a policy!]

Taking quotes from your mail in reversed order:

> All are bad arguments violating API contract,
> all detected at method activation time -> IAE.

Agreed... if the policy is to _always_ check for "null"!

If we sometimes check and sometimes not, the thrown exception will be from a
different hierarchy (NPE vs MathRuntimeException). This is annoying (and
inconsistent).

The oft-repeated issue is "Is it useful to check for null or not?"; at least
4 people answered "No", because this bug will never go unnoticed: sooner or
later, using "null" _will_ raise exception. The sole and unique difference
with CM checking and JVM checking is that the stack trace will (sometimes)
be a little shorter in the former case. Most people have come to agree that
it's not worth adopting a policy of thorough checking. [Rationale 1: users
who encounter a NPE will need to inspect the stack trace and go to _their_
code in order to see where _they_ put a "null". Rationale 2: there is
probably a performance penalty to all these checks (especially in a well
tested code where "null" reference bugs have been ironed out).]

> Consider, e.g. an empty array or array indices that will lead to
> index out of bounds errors in APIs that take arrays.  What is so
> different about null?

There is no difference... if there is a policy that decrees so.

It is _not_ the policy of the JDK: "NullPointerException" is not the same
error as "IndexOutOfBoundsException"; their common parent class is the
(unspecific) "RuntimeException" and their sibling is
"IllegalArgumentException".
There is no "is-a" relationship between NPE, IOBE and IAE.

Recalling that some 1.5 years ago you were against the adoption of the
single exception hierarchy, rooted at "MathRuntimeException", in order to
stay faithful to the JDK exception types, I wonder why you are on the
opposite stand as NPE is concerned.

> [...] what is different about null arguments at the point of
> method activation in an API that explicitly disallows them.

The difference is that there is no need to tell the user what the problem
is because the raised exception says it all: "You tried to use a null
reference." [As said above, the only issue is _when_ the exception is
triggered.]

The policy mandates what is globally valid, e.g.:
  "If not specified otherwise, "null" is not allowed as an argument."
Then, a user cannot complain about a NPE being propagated when he passed an
invalid (null) argument.

Finally, the case for "null" is also slightly peculiar (given the above)
that it should not simply be bundled with the rationale "Fail early": Indeed
"null" references always fail early (i.e. as soon as they are used).
Deferring the check until it is done by the JVM will never entails the code
to produce a wrong result _because_ of that null reference (it will just
fail in the predictable way: NPE).[1]


Gilles

[1] Unlike in C, where an unitialized pointer would not necessarily crash
    the program, but _could_ make it behave erratically (including produce
    wrong results in a stealth way).

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/12/12 5:14 AM, Sébastien Brisard wrote:
> 2012/9/12 Gilles Sadowski <gi...@harfang.homelinux.org>:
>> On Wed, Sep 12, 2012 at 01:14:32PM +0200, Sébastien Brisard wrote:
>>> Hi Gilles,
>>>
>>> 2012/9/12 Gilles Sadowski <gi...@harfang.homelinux.org>:
>>>> On Wed, Sep 12, 2012 at 06:46:29AM +0200, Sébastien Brisard wrote:
>>>>> Hi Phil,
>>>>>
>>>>> 2012/9/10 Phil Steitz <ph...@gmail.com>:
>>>>>> On 9/10/12 11:47 AM, Sébastien Brisard wrote:
>>>>>>> Hi
>>>>>>> What should I do there?
>>>>>>> I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
>>>>>>> throws a NAE. Should I catch it below, and rethrow it with a more
>>>>>>> detailed message (including the entry index)?
>>>>>> IMO, yes.
>>>>>>
>>>>>> I would also check v itself and add to the javadoc contract that IAE
>>>>>> is thrown if v is null.  This is not consistently done in [math],
>>>>>> though, and rarely in the linear package, so I am OK just letting
>>>>>> the NPE propagate if v is null.   It is a little awkward that v
>>>>>> itself being null leads to NPE, but a component of it null leads to
>>>>>> MIAE.
>>>>>>
>>>>> I agree with you, it feels weird. I found a better way: we need to
>>>>> make sure that entries of FieldVector can *never* be null. This means
>>>>> checking for null in setters, constructors and the likes. What do you
>>>>> think?
>>>> That would certainly simplify some code.
>>>> But (devil's advocate) should we consider that some people may rely on the
>>>> possibility to set "null" entries?
>>>>
>>> Yes, didn't think of that. However, the javadoc does not specify that
>>> null is allowed. So referring to earlier discussions, that should mean
>>> that it is forbidden... I'm stretching the argument a little bit, I
>>> know.
>> That's fine with me. In the sense that it is IMHO a _developer_'s choice:
>> It's a statement about the library ("We decide for consistency and
>> robustness reasons, that "null" is forbidden").
>> I contend that it is the same kind of argument that led most of us to prefer
>> immutable instances (even if some users might want to have mutable ones).
>>
> Fine, then. I'll implement null checks in all setters and
> constructors, so that it is guaranteed that null will never be met
> elsewhere.
> I will modify the javadoc of the interface to clearly state that null
> values should not be accepted, and that a MNAE should be thrown in
> that case.

+1 Good idea that adds robustness.

It is not ideal, but as a general principle where the javadoc is
underspecified, it is IMO fair game to explicitly state
preconditions and add checks for them.  This amounts to fully
specifying the API contract.  Code that relies on undocumented
behavior can break in this case.  This is why it is best to specify
the contract as fully as possible.  We need to be kind and practical
when applying this principle though.  Assumptions that *lots of
users* are likely to be making made suddenly no longer true should
be avoided.  The present case is unlikely to be a practical problem
for anyone.

I know this is a little of a sore subject, but in this and other
cases where arguments violate documented preconditions, I think we
should throw IAE, which means MNAE is only appropriate as long as it
continues to subclass our surrogate for IAE - MIAE.

If I sound hard-headed here, it would be great if someone could
explain to me what is different about null arguments at the point of
method activation in an API that explicitly disallows them. 
Consider, e.g. an empty array or array indices that will lead to
index out of bounds errors in APIs that take arrays.  What is so
different about null?  All are bad arguments violating API contract,
all detected at method activation time -> IAE.

Phil
> Sébastien
>
>
> ---------------------------------------------------------------------
> 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] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
2012/9/12 Gilles Sadowski <gi...@harfang.homelinux.org>:
> On Wed, Sep 12, 2012 at 01:14:32PM +0200, Sébastien Brisard wrote:
>> Hi Gilles,
>>
>> 2012/9/12 Gilles Sadowski <gi...@harfang.homelinux.org>:
>> > On Wed, Sep 12, 2012 at 06:46:29AM +0200, Sébastien Brisard wrote:
>> >> Hi Phil,
>> >>
>> >> 2012/9/10 Phil Steitz <ph...@gmail.com>:
>> >> > On 9/10/12 11:47 AM, Sébastien Brisard wrote:
>> >> >> Hi
>> >> >> What should I do there?
>> >> >> I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
>> >> >> throws a NAE. Should I catch it below, and rethrow it with a more
>> >> >> detailed message (including the entry index)?
>> >> >
>> >> > IMO, yes.
>> >> >
>> >> > I would also check v itself and add to the javadoc contract that IAE
>> >> > is thrown if v is null.  This is not consistently done in [math],
>> >> > though, and rarely in the linear package, so I am OK just letting
>> >> > the NPE propagate if v is null.   It is a little awkward that v
>> >> > itself being null leads to NPE, but a component of it null leads to
>> >> > MIAE.
>> >> >
>> >> I agree with you, it feels weird. I found a better way: we need to
>> >> make sure that entries of FieldVector can *never* be null. This means
>> >> checking for null in setters, constructors and the likes. What do you
>> >> think?
>> >
>> > That would certainly simplify some code.
>> > But (devil's advocate) should we consider that some people may rely on the
>> > possibility to set "null" entries?
>> >
>> Yes, didn't think of that. However, the javadoc does not specify that
>> null is allowed. So referring to earlier discussions, that should mean
>> that it is forbidden... I'm stretching the argument a little bit, I
>> know.
>
> That's fine with me. In the sense that it is IMHO a _developer_'s choice:
> It's a statement about the library ("We decide for consistency and
> robustness reasons, that "null" is forbidden").
> I contend that it is the same kind of argument that led most of us to prefer
> immutable instances (even if some users might want to have mutable ones).
>
Fine, then. I'll implement null checks in all setters and
constructors, so that it is guaranteed that null will never be met
elsewhere.
I will modify the javadoc of the interface to clearly state that null
values should not be accepted, and that a MNAE should be thrown in
that case.
Sébastien


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Wed, Sep 12, 2012 at 01:14:32PM +0200, Sébastien Brisard wrote:
> Hi Gilles,
> 
> 2012/9/12 Gilles Sadowski <gi...@harfang.homelinux.org>:
> > On Wed, Sep 12, 2012 at 06:46:29AM +0200, Sébastien Brisard wrote:
> >> Hi Phil,
> >>
> >> 2012/9/10 Phil Steitz <ph...@gmail.com>:
> >> > On 9/10/12 11:47 AM, Sébastien Brisard wrote:
> >> >> Hi
> >> >> What should I do there?
> >> >> I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
> >> >> throws a NAE. Should I catch it below, and rethrow it with a more
> >> >> detailed message (including the entry index)?
> >> >
> >> > IMO, yes.
> >> >
> >> > I would also check v itself and add to the javadoc contract that IAE
> >> > is thrown if v is null.  This is not consistently done in [math],
> >> > though, and rarely in the linear package, so I am OK just letting
> >> > the NPE propagate if v is null.   It is a little awkward that v
> >> > itself being null leads to NPE, but a component of it null leads to
> >> > MIAE.
> >> >
> >> I agree with you, it feels weird. I found a better way: we need to
> >> make sure that entries of FieldVector can *never* be null. This means
> >> checking for null in setters, constructors and the likes. What do you
> >> think?
> >
> > That would certainly simplify some code.
> > But (devil's advocate) should we consider that some people may rely on the
> > possibility to set "null" entries?
> >
> Yes, didn't think of that. However, the javadoc does not specify that
> null is allowed. So referring to earlier discussions, that should mean
> that it is forbidden... I'm stretching the argument a little bit, I
> know.

That's fine with me. In the sense that it is IMHO a _developer_'s choice:
It's a statement about the library ("We decide for consistency and
robustness reasons, that "null" is forbidden").
I contend that it is the same kind of argument that led most of us to prefer
immutable instances (even if some users might want to have mutable ones).


Regards,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hi Gilles,

2012/9/12 Gilles Sadowski <gi...@harfang.homelinux.org>:
> On Wed, Sep 12, 2012 at 06:46:29AM +0200, Sébastien Brisard wrote:
>> Hi Phil,
>>
>> 2012/9/10 Phil Steitz <ph...@gmail.com>:
>> > On 9/10/12 11:47 AM, Sébastien Brisard wrote:
>> >> Hi
>> >> What should I do there?
>> >> I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
>> >> throws a NAE. Should I catch it below, and rethrow it with a more
>> >> detailed message (including the entry index)?
>> >
>> > IMO, yes.
>> >
>> > I would also check v itself and add to the javadoc contract that IAE
>> > is thrown if v is null.  This is not consistently done in [math],
>> > though, and rarely in the linear package, so I am OK just letting
>> > the NPE propagate if v is null.   It is a little awkward that v
>> > itself being null leads to NPE, but a component of it null leads to
>> > MIAE.
>> >
>> I agree with you, it feels weird. I found a better way: we need to
>> make sure that entries of FieldVector can *never* be null. This means
>> checking for null in setters, constructors and the likes. What do you
>> think?
>
> That would certainly simplify some code.
> But (devil's advocate) should we consider that some people may rely on the
> possibility to set "null" entries?
>
Yes, didn't think of that. However, the javadoc does not specify that
null is allowed. So referring to earlier discussions, that should mean
that it is forbidden... I'm stretching the argument a little bit, I
know.
Sébastien


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Wed, Sep 12, 2012 at 06:46:29AM +0200, Sébastien Brisard wrote:
> Hi Phil,
> 
> 2012/9/10 Phil Steitz <ph...@gmail.com>:
> > On 9/10/12 11:47 AM, Sébastien Brisard wrote:
> >> Hi
> >> What should I do there?
> >> I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
> >> throws a NAE. Should I catch it below, and rethrow it with a more
> >> detailed message (including the entry index)?
> >
> > IMO, yes.
> >
> > I would also check v itself and add to the javadoc contract that IAE
> > is thrown if v is null.  This is not consistently done in [math],
> > though, and rarely in the linear package, so I am OK just letting
> > the NPE propagate if v is null.   It is a little awkward that v
> > itself being null leads to NPE, but a component of it null leads to
> > MIAE.
> >
> I agree with you, it feels weird. I found a better way: we need to
> make sure that entries of FieldVector can *never* be null. This means
> checking for null in setters, constructors and the likes. What do you
> think?

That would certainly simplify some code.
But (devil's advocate) should we consider that some people may rely on the
possibility to set "null" entries?


Gilles

> [...]

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi Sébastien.

> 
> 2012/9/11 Gilles Sadowski <gi...@harfang.homelinux.org>:
> > On Mon, Sep 10, 2012 at 08:47:35PM +0200, Sébastien Brisard wrote:
> >> Hi
> >> What should I do there?
> >
> > If we adopt the "flexible" policy (cf. other post), then you can do what you
> > want. ;-)
> >
> This is absolutely what I do not want to do... I've already realized
> that I've been too flexible on e.g. formatting this last year...

The flexible policy wrt null checks is that we concile the possibility to
keep checking (for those who want to), and to not check. And from the user's
perspective, both can be handled in the same way (because it is a NPE[1]
that will be thrown, sooner or later). [From our perspective, CM will also
be consistent albeit, depending on our positions, sometimes incomplete. The
advantage of that policy is that, if we indeed want to fail as early as
possible, we can just incrementally add precondition checks without changing
the overall behaviour of the library: only the stack trace will become
shorter...]


Regards,
Gilles

[1] In CM 4.0, when we can change the inheritance tree for NAE.

> Sébastien
> 
> >> I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
> >> throws a NAE. Should I catch it below, and rethrow it with a more
> >> detailed message (including the entry index)?
> >>
> >> Best,
> >> Sébastien
> >>
> >>
> >>     /** {@inheritDoc} */
> >>     public FieldVector<T> add(FieldVector<T> v)
> >>             throws DimensionMismatchException {
> >>         try {
> >>             return add((ArrayFieldVector<T>) v);
> >>         } catch (ClassCastException cce) {
> >>             checkVectorDimensions(v);
> >>             T[] out = buildArray(data.length);
> >>             for (int i = 0; i < data.length; i++) {
> >>                 out[i] = data[i].add(v.getEntry(i));
> >>                 // SHOULD I CATCH NAE HERE?
> >
> > Not _catch_ NAE but _throw_ it; the line in the loop would become:
> >
> >    final T entry = v.getEntry(i);
> >    if (entry == null) {
> >      throw new NullArgumentException(LocalizedFormats.INDEX, i);
> >    }
> >    out[i] = data[i].add(entry);
> >
> >
> >>             }
> >>             return new ArrayFieldVector<T>(field, out, false);
> >>         }
> >>     }
> >>
> >
> > 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] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Tue, Sep 11, 2012 at 07:46:20AM +0200, Sébastien Brisard wrote:
> Hello,
> 
> 2012/9/11 Gilles Sadowski <gi...@harfang.homelinux.org>:
> > On Mon, Sep 10, 2012 at 01:31:12PM -0700, Phil Steitz wrote:
> >> On 9/10/12 11:47 AM, Sébastien Brisard wrote:
> >> > Hi
> >> > What should I do there?
> >> > I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
> >> > throws a NAE. Should I catch it below, and rethrow it with a more
> >> > detailed message (including the entry index)?
> >>
> >> IMO, yes.
> >>
> >> I would also check v itself and add to the javadoc contract that IAE
> >> is thrown if v is null.  This is not consistently done in [math],
> >> though, and rarely in the linear package, so I am OK just letting
> >> the NPE propagate if v is null.   It is a little awkward that v
> >> itself being null leads to NPE, but a component of it null leads to
> >> MIAE.
> >
> > Awkward? Of course it is; that's what I explained two posts ago.
> >
> > If we want to allow for the possibility of checking for null (and I agree
> > that it could be useful to pinpoint the problem by passing the information
> > about which index contains an invalid null entry), then we should adopt the
> > second option which I presented in that preceding post:
> >
> >   "NullArgumentException" inherits from "NullPointerException"
> >
> > This really solves all issues (now that Luc has said that it is not a
> > problem if this one exception escapes the single-root hierarchy): It allows
> > to check for null in CM code and raise the same kind of exception that would
> > arise when no null check is performed. Both flexible and consistent.
> >
> I may be a bit slow, but I understood that localized error messages
> were an absolute requirement on Luc's side (which, BTW is good to
> know, I have always been wondering why we cared so much about
> localized error messages...).

As I've already expressed in the past, I think that localization (as
required by some of Luc's applications), as well as the more recent example
of passing "null" to some constructors (as provided by Phil) are indeed
to be considered as _outside_ the realm of CM: They are application
requirements, pushed down to CM, that cannot be given a self-consistent
rationale.
When adding a feature to a library like CM, I think that it is very
important (namely for long-term maintenance) that it does not rely on
"out-of-band" information.  I mean that the "Why does this feature
exist?" should not need references to external-only requirements to be
understandable.

Of course, by "external-only" I mean that the feature's implementation
could be changed just because the applications that use it changes.
E.g. if the "TranslatorService", which I imagined in a previous post,
existed, Luc's application could use it to fulfill its "Error messages
should be displayed in French" requirement, and the localization of CM
error messages could be removed from CM, without CM loosing anything.

In that sense, the support of localization is not on the same footing as
the support of "<some math algorithm>" because the latter is within the core
business of CM, while the former is not.

IMHO, the more a codebase grows, the less it should contain internal
inconsistencies.
Of course, we can conclude that now is not the right moment to clean up
this or other defects of CM (e.g. because nobody has the time to do it, or
to please people who have come to rely on the current state of affairs) but
that is not making the issue go away.

> So, if I understand correctly, NAE inheriting from NPE would mean no
> error message.

No.

> In the present case, if we can't specify the index of
> the faulty entry in the error message,

You can:

 throw new NullArgumentException(LocalizedFormats.INDEX, i);

> I don't see the benefit of NAE
> over NPE. I must have missed something; I guess that NAE would inherit
> from NPE, AND implement ExceptionContextProvider, is that right? In
> that case, I like this solution.

Exactly.
The _sole_ difference with the current NAE will be the change in the
extends clause, from

  public class NullArgumentException extends MathIllegalArgumentException

to

  public class NullArgumentException extends NullPointerException
  implements ExceptionContextProvider

[And the "throw" statement above will be the same before and after this
change.]

> Sorry for polluting this thread with silly questions, but I am not
> used to writing programs for third-party end-users (I *AM* the
> end-user, or maybe my student enxt door), so I have to say I'm not
> familiar with the concerns raised in this thread.

They are not silly questions. It's precisely my point that such questions
will arise (because of CM is lacking self-consistency).

I'm pretty sure that none of us is familiar with being only on one side of
the border; we all are also _users_ of CM and, quite often, the user's
point-of-view is given the priority even if it leads to decisions that are
sub-optimal in the longer term.


Best regards,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hello,

2012/9/11 Gilles Sadowski <gi...@harfang.homelinux.org>:
> On Mon, Sep 10, 2012 at 01:31:12PM -0700, Phil Steitz wrote:
>> On 9/10/12 11:47 AM, Sébastien Brisard wrote:
>> > Hi
>> > What should I do there?
>> > I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
>> > throws a NAE. Should I catch it below, and rethrow it with a more
>> > detailed message (including the entry index)?
>>
>> IMO, yes.
>>
>> I would also check v itself and add to the javadoc contract that IAE
>> is thrown if v is null.  This is not consistently done in [math],
>> though, and rarely in the linear package, so I am OK just letting
>> the NPE propagate if v is null.   It is a little awkward that v
>> itself being null leads to NPE, but a component of it null leads to
>> MIAE.
>
> Awkward? Of course it is; that's what I explained two posts ago.
>
> If we want to allow for the possibility of checking for null (and I agree
> that it could be useful to pinpoint the problem by passing the information
> about which index contains an invalid null entry), then we should adopt the
> second option which I presented in that preceding post:
>
>   "NullArgumentException" inherits from "NullPointerException"
>
> This really solves all issues (now that Luc has said that it is not a
> problem if this one exception escapes the single-root hierarchy): It allows
> to check for null in CM code and raise the same kind of exception that would
> arise when no null check is performed. Both flexible and consistent.
>
I may be a bit slow, but I understood that localized error messages
were an absolute requirement on Luc's side (which, BTW is good to
know, I have always been wondering why we cared so much about
localized error messages...).
So, if I understand correctly, NAE inheriting from NPE would mean no
error message. In the present case, if we can't specify the index of
the faulty entry in the error message, I don't see the benefit of NAE
over NPE. I must have missed something; I guess that NAE would inherit
from NPE, AND implement ExceptionContextProvider, is that right? In
that case, I like this solution.
Sorry for polluting this thread with silly questions, but I am not
used to writing programs for third-party end-users (I *AM* the
end-user, or maybe my student enxt door), so I have to say I'm not
familiar with the concerns raised in this thread.

Sébastien


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Mon, Sep 10, 2012 at 01:31:12PM -0700, Phil Steitz wrote:
> On 9/10/12 11:47 AM, Sébastien Brisard wrote:
> > Hi
> > What should I do there?
> > I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
> > throws a NAE. Should I catch it below, and rethrow it with a more
> > detailed message (including the entry index)?
> 
> IMO, yes.
> 
> I would also check v itself and add to the javadoc contract that IAE
> is thrown if v is null.  This is not consistently done in [math],
> though, and rarely in the linear package, so I am OK just letting
> the NPE propagate if v is null.   It is a little awkward that v
> itself being null leads to NPE, but a component of it null leads to
> MIAE.

Awkward? Of course it is; that's what I explained two posts ago.

If we want to allow for the possibility of checking for null (and I agree
that it could be useful to pinpoint the problem by passing the information
about which index contains an invalid null entry), then we should adopt the
second option which I presented in that preceding post:

  "NullArgumentException" inherits from "NullPointerException"

This really solves all issues (now that Luc has said that it is not a
problem if this one exception escapes the single-root hierarchy): It allows
to check for null in CM code and raise the same kind of exception that would
arise when no null check is performed. Both flexible and consistent.


Gilles

> [...]

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/10/12 3:46 PM, Gilles Sadowski wrote:
> On Mon, Sep 10, 2012 at 08:47:35PM +0200, Sébastien Brisard wrote:
>> Hi
>> What should I do there?
> If we adopt the "flexible" policy (cf. other post), then you can do what you
> want. ;-)

Good one :)
>
>> I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
>> throws a NAE. Should I catch it below, and rethrow it with a more
>> detailed message (including the entry index)?
>>
>> Best,
>> Sébastien
>>
>>
>>     /** {@inheritDoc} */
>>     public FieldVector<T> add(FieldVector<T> v)
>>             throws DimensionMismatchException {
>>         try {
>>             return add((ArrayFieldVector<T>) v);
>>         } catch (ClassCastException cce) {
>>             checkVectorDimensions(v);
>>             T[] out = buildArray(data.length);
>>             for (int i = 0; i < data.length; i++) {
>>                 out[i] = data[i].add(v.getEntry(i));
>>                 // SHOULD I CATCH NAE HERE?
> Not _catch_ NAE but _throw_ it; the line in the loop would become:
>
>    final T entry = v.getEntry(i);
>    if (entry == null) {
>      throw new NullArgumentException(LocalizedFormats.INDEX, i);
>    }
>    out[i] = data[i].add(entry);

What about for v itself?

Phil
>
>
>>             }
>>             return new ArrayFieldVector<T>(field, out, false);
>>         }
>>     }
>>
> 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] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hi,

2012/9/11 Gilles Sadowski <gi...@harfang.homelinux.org>:
> On Mon, Sep 10, 2012 at 08:47:35PM +0200, Sébastien Brisard wrote:
>> Hi
>> What should I do there?
>
> If we adopt the "flexible" policy (cf. other post), then you can do what you
> want. ;-)
>
This is absolutely what I do not want to do... I've already realized
that I've been too flexible on e.g. formatting this last year...
Sébastien

>> I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
>> throws a NAE. Should I catch it below, and rethrow it with a more
>> detailed message (including the entry index)?
>>
>> Best,
>> Sébastien
>>
>>
>>     /** {@inheritDoc} */
>>     public FieldVector<T> add(FieldVector<T> v)
>>             throws DimensionMismatchException {
>>         try {
>>             return add((ArrayFieldVector<T>) v);
>>         } catch (ClassCastException cce) {
>>             checkVectorDimensions(v);
>>             T[] out = buildArray(data.length);
>>             for (int i = 0; i < data.length; i++) {
>>                 out[i] = data[i].add(v.getEntry(i));
>>                 // SHOULD I CATCH NAE HERE?
>
> Not _catch_ NAE but _throw_ it; the line in the loop would become:
>
>    final T entry = v.getEntry(i);
>    if (entry == null) {
>      throw new NullArgumentException(LocalizedFormats.INDEX, i);
>    }
>    out[i] = data[i].add(entry);
>
>
>>             }
>>             return new ArrayFieldVector<T>(field, out, false);
>>         }
>>     }
>>
>
> 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] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Mon, Sep 10, 2012 at 08:47:35PM +0200, Sébastien Brisard wrote:
> Hi
> What should I do there?

If we adopt the "flexible" policy (cf. other post), then you can do what you
want. ;-)

> I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
> throws a NAE. Should I catch it below, and rethrow it with a more
> detailed message (including the entry index)?
> 
> Best,
> Sébastien
> 
> 
>     /** {@inheritDoc} */
>     public FieldVector<T> add(FieldVector<T> v)
>             throws DimensionMismatchException {
>         try {
>             return add((ArrayFieldVector<T>) v);
>         } catch (ClassCastException cce) {
>             checkVectorDimensions(v);
>             T[] out = buildArray(data.length);
>             for (int i = 0; i < data.length; i++) {
>                 out[i] = data[i].add(v.getEntry(i));
>                 // SHOULD I CATCH NAE HERE?

Not _catch_ NAE but _throw_ it; the line in the loop would become:

   final T entry = v.getEntry(i);
   if (entry == null) {
     throw new NullArgumentException(LocalizedFormats.INDEX, i);
   }
   out[i] = data[i].add(entry);


>             }
>             return new ArrayFieldVector<T>(field, out, false);
>         }
>     }
> 

Regards,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by James Ring <sj...@jdns.org>.
Hey Phil

On Mon, Sep 10, 2012 at 5:09 PM, Phil Steitz <ph...@gmail.com> wrote:
> On 9/10/12 3:43 PM, James Ring wrote:
>>>  try {
>>>    // Call CM
>>>  } catch (NullPointerException e) {
>>>    // Handle NPE (raised by the JVM _or_ by CM).
>>>  }
>> Is this going to be the typical case? NPE indicates a programming
>> error, so this is something that *should* completely crash your JVM.
>
> Maybe I am being stupid here, but why is NPE so vastly different
> from, say, array index error and why is it bad to check parameters
> against documented preconditions and throw IAE when actual
> parameters do not meet the preconditions?   Do you think APIs should
> just "let the chips fall where they may" regarding invalid
> arguments, or do you think checking parameters and throwing IAE with
> an informative message is an acceptable practice?

I don't think NPE is different, you should check parameters against
preconditions and throw IAE as soon as you can. APIs should fail fast
so the stack traces are meaningful and errors are caught as early as
possible. Throwing IAE with an informative error is great.

Throwing an exception with a localized message seems pointless to me, FWIW.

> Phil

Regards,
James

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/10/12 3:43 PM, James Ring wrote:
> Hey,
>
> I'm a disinterested third party (not a CM user) but I thought I should
> chime in my two cents worth...
>
> On Sun, Sep 9, 2012 at 4:34 AM, Gilles Sadowski
> <gi...@harfang.homelinux.org> wrote:
>> Hi.
>>
>> Further discussion on the JIRA page
>>   https://issues.apache.org/jira/browse/MATH-856
>> cannot reach a consensus on solving the issue that raised this thread.
>>
>> The proposal was that CM never checks for "null" and lets the JVM do it (and
>> thus throw the standard NPE).
> I think a standard NPE should be thrown as soon as a null is detected
> where one shouldn't be. If the caller passes in a null and the callee
> knows that's bad, throw the NPE right there. Fail fast.
>
>> Phil wants to retain some null checks but opposes to throwing a NPE without
>> a "detailed message".
>> The localization mechanism being implemented in "ExceptionContext", we
>> cannot throw a standard NPE (since the error message won't be localized).
> I personally think that localizing exceptions is madness. Maybe that's
> just me. Why not localize the CM source code as well? L10n is a
> problem for application writers and not math libraries.
>
> It seems to me that CM is trying to achieve documentation through
> exceptions, as if the exceptions thrown should guide the user how to
> use the library. This is the job of javadoc, tutorials, wikis, etc.
>
>> For a consistent behaviour (as seen from the caller), we would have to
>> implement a subclass of the standard NPE: callers could do
>>
>>  try {
>>    // Call CM
>>  } catch (NullPointerException e) {
>>    // Handle NPE (raised by the JVM _or_ by CM).
>>  }
> Is this going to be the typical case? NPE indicates a programming
> error, so this is something that *should* completely crash your JVM.

Maybe I am being stupid here, but why is NPE so vastly different
from, say, array index error and why is it bad to check parameters
against documented preconditions and throw IAE when actual
parameters do not meet the preconditions?   Do you think APIs should
just "let the chips fall where they may" regarding invalid
arguments, or do you think checking parameters and throwing IAE with
an informative message is an acceptable practice?

Phil
>
>> However, this breaks the consensus we arrived at (for v4.0) about CM
>> throwing only subclasses of "MathRuntimeExceprion" (singly rooted hierarchy).
>>
>> Phil proposes to throw MathIAE (IMO, it must be the specific
>> "NullArgumentException"), but this breaks the above use-case: Users have to
>> do
>>
>>  try {
>>    // Call CM
>>  } catch (NullPointerException e) {
>>    // Handle NPE (raised by the JVM).
>>  } catch (NullArgumentException e)
>>    // Handle NPE (raised by CM).
>>  }
>>
>> showing blatantly that CM is not consistent: sometimes it lets a JVM
>> exception propagate, and sometimes it catches the problem eralier and throws
>> an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0,
>> "MathRuntimeException").
>> This is the current state of affairs, and I think that it is not
>> satisfactory. [As proven by this issue having recurred two or three times
>> already.]
> +1 this is terrible from an API design perspective
>
>> In light of this, I propose that either
>> * Phil changes his mind (no check for null performed in CM code), or
>> * we make an exception to the singly-rooted hierarchy just for
>>   "NullArgumentException" (which, in 4.0, would become a subclass of the
>>   standard NPE).
>>
>> The second option cares for all the various positions _except_ the
>> singly-rooted hierarchy.
>>
>>
>> Regards,
>> Gilles
> Hope you can all work it out. I would start by dropping the
> document-by-exception thinking that seems to permeate these
> discussions.
>
> Regards,
> James
>
> ---------------------------------------------------------------------
> 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] About "NullArgumentException"

Posted by James Ring <sj...@jdns.org>.
Hey,

I'm a disinterested third party (not a CM user) but I thought I should
chime in my two cents worth...

On Sun, Sep 9, 2012 at 4:34 AM, Gilles Sadowski
<gi...@harfang.homelinux.org> wrote:
> Hi.
>
> Further discussion on the JIRA page
>   https://issues.apache.org/jira/browse/MATH-856
> cannot reach a consensus on solving the issue that raised this thread.
>
> The proposal was that CM never checks for "null" and lets the JVM do it (and
> thus throw the standard NPE).

I think a standard NPE should be thrown as soon as a null is detected
where one shouldn't be. If the caller passes in a null and the callee
knows that's bad, throw the NPE right there. Fail fast.

> Phil wants to retain some null checks but opposes to throwing a NPE without
> a "detailed message".
> The localization mechanism being implemented in "ExceptionContext", we
> cannot throw a standard NPE (since the error message won't be localized).

I personally think that localizing exceptions is madness. Maybe that's
just me. Why not localize the CM source code as well? L10n is a
problem for application writers and not math libraries.

It seems to me that CM is trying to achieve documentation through
exceptions, as if the exceptions thrown should guide the user how to
use the library. This is the job of javadoc, tutorials, wikis, etc.

> For a consistent behaviour (as seen from the caller), we would have to
> implement a subclass of the standard NPE: callers could do
>
>  try {
>    // Call CM
>  } catch (NullPointerException e) {
>    // Handle NPE (raised by the JVM _or_ by CM).
>  }

Is this going to be the typical case? NPE indicates a programming
error, so this is something that *should* completely crash your JVM.

> However, this breaks the consensus we arrived at (for v4.0) about CM
> throwing only subclasses of "MathRuntimeExceprion" (singly rooted hierarchy).
>
> Phil proposes to throw MathIAE (IMO, it must be the specific
> "NullArgumentException"), but this breaks the above use-case: Users have to
> do
>
>  try {
>    // Call CM
>  } catch (NullPointerException e) {
>    // Handle NPE (raised by the JVM).
>  } catch (NullArgumentException e)
>    // Handle NPE (raised by CM).
>  }
>
> showing blatantly that CM is not consistent: sometimes it lets a JVM
> exception propagate, and sometimes it catches the problem eralier and throws
> an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0,
> "MathRuntimeException").
> This is the current state of affairs, and I think that it is not
> satisfactory. [As proven by this issue having recurred two or three times
> already.]

+1 this is terrible from an API design perspective

> In light of this, I propose that either
> * Phil changes his mind (no check for null performed in CM code), or
> * we make an exception to the singly-rooted hierarchy just for
>   "NullArgumentException" (which, in 4.0, would become a subclass of the
>   standard NPE).
>
> The second option cares for all the various positions _except_ the
> singly-rooted hierarchy.
>
>
> Regards,
> Gilles

Hope you can all work it out. I would start by dropping the
document-by-exception thinking that seems to permeate these
discussions.

Regards,
James

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/10/12 3:44 AM, Gilles Sadowski wrote:
>>> [...]
>>>
>>> P.S. Is there an occurrence in CM, where a method can be passed a null
>>>      argument?
>> Yes.   One example is the constructor for EmpiricalDistribution that
>> takes a RandomGenerator as argument.
> Thanks for finding one of those few examples.
> The first remark (concerning the issue at hand in this thread) is that it is
> enough to say in the documentation of those cases that "null" is allowed.

Good, we agree there.
>
> The policy is (would be) that "null" is never a valid argument (except in
> the documented cases).

I agree there.  Where we disagree is what we should do when an
illegal null argument is passed to an API that does not accept
nulls.  I like to document and throw IAE in this case (per
guidelines in the Developers Guide).  That is more robust, since
failure is immediate, there are no potential downstream impacts /
resource utilization / leakage potential, and it is easier to debug
/ trace without looking at the source code, especially if an
informative message indicating what precondition failed is provided.
>
>> If a null is supplied, the
>> constructor does not complain and the lazy initialization works as
>> though the argumentless constructor had been used and a JDK random
>> generator is created.
> The second remark is that allowing "null" in this case only brings confusion
> because there also exists overloaded constructors. [Overloading is the right
> way to provide default arguments; using "null" is to be avoided (and is a
> remnant from C programming where no two methods can have the same name).]

I am sure you can figure out how to refactor my applications that
use this functionality, but here an example showing why it exists. 
When initializing an application from external configuration
meta-data, if a RandomGenerator class is provided in the config, the
initialization framework creates an instance of the provided class
and sets a property of an application class being created equal to
the instance.  This property is used by constructors of other
members of the class.  If no config is provided for the
RandomGenerator, the property is null.  The null-safe constructor
allows the property to be used as an argument to the constructor,
avoiding the need to code (or configure) the test for null in
initialization.

>
> We thus have
>  public EmpiricalDistribution(int binCount)
> and
>  public EmpiricalDistribution(int binCount, RandomGenerator generator)
>
> In the latter case, the doc says:
>      * @param generator random data generator (may be null, resulting in
>      * default JDK generator)
>
> Which is also what happens in the former case, albeit a little later.
>
> It's confusing because the word "lazy" does not appear anywhere, so there is
> no usefulness (from the user's point-of-view) to pass null to the two-args
> constructor, where he could use the more concise, one-arg, constructor.

See use case above - when what is being passed in is a property,
which may be null, it is convenient to be able to provide null to
the constructor.

Phil

>
> Even if the doc would highlight the difference, it's still needlessly
> complicated to allow "null" since the class's seemingly central method
> "getNextValue" would trigger the initialization of the RNG sooner or later.
> [Hence the gain of "lazy init" is nil.]
>
> If there are use-cases where the RNG part of the functionality is never
> used, then, IMHO, the design can be improved by separating the data loading
> + statistics computation from the actual distribution functionality
> ("getNextValue").
> In the new product, there would be no reference to a RNG (thus vanished the
> "can be null, and if it is, there will be lazy init etc.") in one class,
> while in the other, "getNextValue" would indeed be so prominent that the
> class cannot be used without a RNG (thus lazy init is totally useless) and
> that argument _cannot_ be null.
>
>> There are other similar examples, mostly
>> constructors, IIRC.
> Then a similar analysis probably applies.
>
>
> Gilles
>
>> Phil
>>>
>>>> Phil
>>>>
>>>>> The second option cares for all the various positions _except_ the
>>>>> singly-rooted hierarchy.
>>>>>
>>>>>
>>>>> Regards,
>>>>> Gilles
>>>>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> > [...]
> >
> > P.S. Is there an occurrence in CM, where a method can be passed a null
> >      argument?
> 
> Yes.   One example is the constructor for EmpiricalDistribution that
> takes a RandomGenerator as argument.

Thanks for finding one of those few examples.
The first remark (concerning the issue at hand in this thread) is that it is
enough to say in the documentation of those cases that "null" is allowed.

The policy is (would be) that "null" is never a valid argument (except in
the documented cases).

> If a null is supplied, the
> constructor does not complain and the lazy initialization works as
> though the argumentless constructor had been used and a JDK random
> generator is created.

The second remark is that allowing "null" in this case only brings confusion
because there also exists overloaded constructors. [Overloading is the right
way to provide default arguments; using "null" is to be avoided (and is a
remnant from C programming where no two methods can have the same name).]

We thus have
 public EmpiricalDistribution(int binCount)
and
 public EmpiricalDistribution(int binCount, RandomGenerator generator)

In the latter case, the doc says:
     * @param generator random data generator (may be null, resulting in
     * default JDK generator)

Which is also what happens in the former case, albeit a little later.

It's confusing because the word "lazy" does not appear anywhere, so there is
no usefulness (from the user's point-of-view) to pass null to the two-args
constructor, where he could use the more concise, one-arg, constructor.

Even if the doc would highlight the difference, it's still needlessly
complicated to allow "null" since the class's seemingly central method
"getNextValue" would trigger the initialization of the RNG sooner or later.
[Hence the gain of "lazy init" is nil.]

If there are use-cases where the RNG part of the functionality is never
used, then, IMHO, the design can be improved by separating the data loading
+ statistics computation from the actual distribution functionality
("getNextValue").
In the new product, there would be no reference to a RNG (thus vanished the
"can be null, and if it is, there will be lazy init etc.") in one class,
while in the other, "getNextValue" would indeed be so prominent that the
class cannot be used without a RNG (thus lazy init is totally useless) and
that argument _cannot_ be null.

> There are other similar examples, mostly
> constructors, IIRC.

Then a similar analysis probably applies.


Gilles

> Phil
> >
> >
> >> Phil
> >>
> >>> The second option cares for all the various positions _except_ the
> >>> singly-rooted hierarchy.
> >>>
> >>>
> >>> Regards,
> >>> Gilles
> >>>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Ole Ersoy <ol...@gmail.com>.
>
> Yes.   One example is the constructor for EmpiricalDistribution that
> takes a RandomGenerator as argument.  If a null is supplied, the
> constructor does not complain and the lazy initialization works as
> though the argumentless constructor had been used and a JDK random
> generator is created.  There are other similar examples, mostly
> constructors, IIRC.
>
> Phil

Just a thought.  What if the instead of binding a generator to the Distribution instances, a uniform double or array of uniform doubles were provided?

Cheers,
Ole

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/9/12 3:26 PM, Gilles Sadowski wrote:
> On Sun, Sep 09, 2012 at 09:16:51AM -0700, Phil Steitz wrote:
>> On 9/9/12 4:34 AM, Gilles Sadowski wrote:
>>> Hi.
>>>
>>> Further discussion on the JIRA page
>>>   https://issues.apache.org/jira/browse/MATH-856
>>> cannot reach a consensus on solving the issue that raised this thread.
>>>
>>> The proposal was that CM never checks for "null" and lets the JVM do it (and
>>> thus throw the standard NPE).
>>>
>>> Phil wants to retain some null checks but opposes to throwing a NPE without
>>> a "detailed message".
>>> The localization mechanism being implemented in "ExceptionContext", we
>>> cannot throw a standard NPE (since the error message won't be localized).
>>>
>>> For a consistent behaviour (as seen from the caller), we would have to
>>> implement a subclass of the standard NPE: callers could do
>>>
>>>  try {
>>>    // Call CM
>>>  } catch (NullPointerException e) {
>>>    // Handle NPE (raised by the JVM _or_ by CM).
>>>  }
>>>
>>> However, this breaks the consensus we arrived at (for v4.0) about CM
>>> throwing only subclasses of "MathRuntimeExceprion" (singly rooted hierarchy).
>>>
>>> Phil proposes to throw MathIAE (IMO, it must be the specific
>>> "NullArgumentException"), but this breaks the above use-case: Users have to
>>> do
>>>
>>>  try {
>>>    // Call CM
>>>  } catch (NullPointerException e) {
>>>    // Handle NPE (raised by the JVM).
>>>  } catch (NullArgumentException e)
>>>    // Handle NPE (raised by CM).
>>>  }
>>>
>>> showing blatantly that CM is not consistent: sometimes it lets a JVM
>>> exception propagate, and sometimes it catches the problem eralier and throws
>>> an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0,
>>> "MathRuntimeException").
>>> This is the current state of affairs, and I think that it is not
>>> satisfactory. [As proven by this issue having recurred two or three times
>>> already.]
>>>
>>> In light of this, I propose that either
>>> * Phil changes his mind (no check for null performed in CM code), or
>>> * we make an exception to the singly-rooted hierarchy just for
>>>   "NullArgumentException" (which, in 4.0, would become a subclass of the
>>>   standard NPE).
>> Why not just leave things alone [...]
> For the reason I gave above: the inconsistent/non-existent policy will make
> the issue recur sooner or later, as it happened now with Sébastien, as it
> happened with me when I first signalled the burden of checked excpetions and
> later when we agreed about MathRuntimeException, then changed again, to come
> back again now, to where we were almost two years ago (IIRC)...
>
>> - i.e., let some APIs document null
>> handling and throw IAE at the point of method invocation when
>> supplying a null violates the documented API contract?
> The answer to that question is in the previous post.
>
>> We can leave the (needless, IMO) NullArgumentException as a subclass
>> of MathIAE in place, or drop it and throw MathIAE directly in these
>> cases.
> "NullArgumentException" is about as needless as any subclass of "Exception"
> or "RuntimeException". Either we use inheritance for what it's primarily
> meant or we choose another, non-OO, language.
>
> This is going in circles; some people will drop from the discussion (or
> already did) and some time from now, someone will "rediscover" this, among
> other little defects of CM. These are worth correcting, but not worth
> discussing endlessly.
>
> Let's just have all people here provide their preference and be done with
> it.
>
>
> Gilles
>
> P.S. Is there an occurrence in CM, where a method can be passed a null
>      argument?

Yes.   One example is the constructor for EmpiricalDistribution that
takes a RandomGenerator as argument.  If a null is supplied, the
constructor does not complain and the lazy initialization works as
though the argumentless constructor had been used and a JDK random
generator is created.  There are other similar examples, mostly
constructors, IIRC.

Phil
>
>
>> Phil
>>
>>> The second option cares for all the various positions _except_ the
>>> singly-rooted hierarchy.
>>>
>>>
>>> Regards,
>>> Gilles
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hi Phil,

2012/9/10 Phil Steitz <ph...@gmail.com>:
> On 9/10/12 11:47 AM, Sébastien Brisard wrote:
>> Hi
>> What should I do there?
>> I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
>> throws a NAE. Should I catch it below, and rethrow it with a more
>> detailed message (including the entry index)?
>
> IMO, yes.
>
> I would also check v itself and add to the javadoc contract that IAE
> is thrown if v is null.  This is not consistently done in [math],
> though, and rarely in the linear package, so I am OK just letting
> the NPE propagate if v is null.   It is a little awkward that v
> itself being null leads to NPE, but a component of it null leads to
> MIAE.
>
I agree with you, it feels weird. I found a better way: we need to
make sure that entries of FieldVector can *never* be null. This means
checking for null in setters, constructors and the likes. What do you
think?
Sébastien
> Phil
>>
>> Best,
>> Sébastien
>>
>>
>>     /** {@inheritDoc} */
>>     public FieldVector<T> add(FieldVector<T> v)
>>             throws DimensionMismatchException {
>>         try {
>>             return add((ArrayFieldVector<T>) v);
>>         } catch (ClassCastException cce) {
>>             checkVectorDimensions(v);
>>             T[] out = buildArray(data.length);
>>             for (int i = 0; i < data.length; i++) {
>>                 out[i] = data[i].add(v.getEntry(i));
>>                 // SHOULD I CATCH NAE HERE?
>>             }
>>             return new ArrayFieldVector<T>(field, out, false);
>>         }
>>     }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/10/12 11:47 AM, Sébastien Brisard wrote:
> Hi
> What should I do there?
> I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
> throws a NAE. Should I catch it below, and rethrow it with a more
> detailed message (including the entry index)?

IMO, yes.

I would also check v itself and add to the javadoc contract that IAE
is thrown if v is null.  This is not consistently done in [math],
though, and rarely in the linear package, so I am OK just letting
the NPE propagate if v is null.   It is a little awkward that v
itself being null leads to NPE, but a component of it null leads to
MIAE.

Phil
>
> Best,
> Sébastien
>
>
>     /** {@inheritDoc} */
>     public FieldVector<T> add(FieldVector<T> v)
>             throws DimensionMismatchException {
>         try {
>             return add((ArrayFieldVector<T>) v);
>         } catch (ClassCastException cce) {
>             checkVectorDimensions(v);
>             T[] out = buildArray(data.length);
>             for (int i = 0; i < data.length; i++) {
>                 out[i] = data[i].add(v.getEntry(i));
>                 // SHOULD I CATCH NAE HERE?
>             }
>             return new ArrayFieldVector<T>(field, out, false);
>         }
>     }
>
>
> ---------------------------------------------------------------------
> 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] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hi
What should I do there?
I'm trying to work on MATH-854. It turns out that FieldElement<T>.add
throws a NAE. Should I catch it below, and rethrow it with a more
detailed message (including the entry index)?

Best,
Sébastien


    /** {@inheritDoc} */
    public FieldVector<T> add(FieldVector<T> v)
            throws DimensionMismatchException {
        try {
            return add((ArrayFieldVector<T>) v);
        } catch (ClassCastException cce) {
            checkVectorDimensions(v);
            T[] out = buildArray(data.length);
            for (int i = 0; i < data.length; i++) {
                out[i] = data[i].add(v.getEntry(i));
                // SHOULD I CATCH NAE HERE?
            }
            return new ArrayFieldVector<T>(field, out, false);
        }
    }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 10/09/2012 08:11, Sébastien Brisard a écrit :
> Hi,
> 
> 2012/9/10 Gilles Sadowski <gi...@harfang.homelinux.org>:
>> On Sun, Sep 09, 2012 at 09:16:51AM -0700, Phil Steitz wrote:
>>> On 9/9/12 4:34 AM, Gilles Sadowski wrote:
>>>> Hi.
>>>>
>>>> Further discussion on the JIRA page
>>>>   https://issues.apache.org/jira/browse/MATH-856
>>>> cannot reach a consensus on solving the issue that raised this thread.
>>>>
>>>> The proposal was that CM never checks for "null" and lets the JVM do it (and
>>>> thus throw the standard NPE).
>>>>
>>>> Phil wants to retain some null checks but opposes to throwing a NPE without
>>>> a "detailed message".
>>>> The localization mechanism being implemented in "ExceptionContext", we
>>>> cannot throw a standard NPE (since the error message won't be localized).
>>>>
>>>> For a consistent behaviour (as seen from the caller), we would have to
>>>> implement a subclass of the standard NPE: callers could do
>>>>
>>>>  try {
>>>>    // Call CM
>>>>  } catch (NullPointerException e) {
>>>>    // Handle NPE (raised by the JVM _or_ by CM).
>>>>  }
>>>>
>>>> However, this breaks the consensus we arrived at (for v4.0) about CM
>>>> throwing only subclasses of "MathRuntimeExceprion" (singly rooted hierarchy).
>>>>
>>>> Phil proposes to throw MathIAE (IMO, it must be the specific
>>>> "NullArgumentException"), but this breaks the above use-case: Users have to
>>>> do
>>>>
>>>>  try {
>>>>    // Call CM
>>>>  } catch (NullPointerException e) {
>>>>    // Handle NPE (raised by the JVM).
>>>>  } catch (NullArgumentException e)
>>>>    // Handle NPE (raised by CM).
>>>>  }
>>>>
>>>> showing blatantly that CM is not consistent: sometimes it lets a JVM
>>>> exception propagate, and sometimes it catches the problem eralier and throws
>>>> an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0,
>>>> "MathRuntimeException").
>>>> This is the current state of affairs, and I think that it is not
>>>> satisfactory. [As proven by this issue having recurred two or three times
>>>> already.]
>>>>
>>>> In light of this, I propose that either
>>>> * Phil changes his mind (no check for null performed in CM code), or
>>>> * we make an exception to the singly-rooted hierarchy just for
>>>>   "NullArgumentException" (which, in 4.0, would become a subclass of the
>>>>   standard NPE).
>>>
>>> Why not just leave things alone [...]
>>
>> For the reason I gave above: the inconsistent/non-existent policy will make
>> the issue recur sooner or later, as it happened now with Sébastien, as it
>> happened with me when I first signalled the burden of checked excpetions and
>> later when we agreed about MathRuntimeException, then changed again, to come
>> back again now, to where we were almost two years ago (IIRC)...
>>
>>> - i.e., let some APIs document null
>>> handling and throw IAE at the point of method invocation when
>>> supplying a null violates the documented API contract?
>>
>> The answer to that question is in the previous post.
>>
>>> We can leave the (needless, IMO) NullArgumentException as a subclass
>>> of MathIAE in place, or drop it and throw MathIAE directly in these
>>> cases.
>>
>> "NullArgumentException" is about as needless as any subclass of "Exception"
>> or "RuntimeException". Either we use inheritance for what it's primarily
>> meant or we choose another, non-OO, language.
>>
>> This is going in circles; some people will drop from the discussion (or
>> already did) and some time from now, someone will "rediscover" this, among
>> other little defects of CM. These are worth correcting, but not worth
>> discussing endlessly.
>>
>> Let's just have all people here provide their preference and be done with
>> it.
>>
> Since there is no way to enforce a strict policy on checking for null
> in CM, I think that NAE is useless, and should be droped.
> If we assume that every user of CM can use a debugger, then probably
> (and contrary to what I advocated earlier), checking early for null is
> also superfluous.

There are some cases pointed to be Phil which can be useful. We can let
them if they are already there.

> I tend to document arguments which *can* be null (I think someone else
> also mentioned this practice), so that it's fairly safe (as someone
> already wrote) to assume that all other arguments *must* be non-null.

+1.

> 
> To sum up, I would favor complete removal of NAE. As for existing
> checks, I would either remove them, or throw an argumentless standard
> NPE.

As null is so ubiquitous, I would tend to choose Gilles second option
(i.e. we preserve NullArgumentException, outside of the single root
hierarchy I promote for other exceptions). This would preserve
robustness Phil asks for and I would not complain against this as in
this case I would not mind if it is not advertised in javadocs and
throws declarations (because even if we don't throw it ourselves, a
regular NPE from JVM can always occur).

> 
> Phil was talking about loss of robustness. I don't think that CM as a
> whole is robust with respect to null pointers.

It is not globally robust, but in some places it is because some effort
was already put on this.

Luc

> In some places, the
> code fails in the standard way (NPE), while in others, it fails in a
> fully documented way. Since this is inconsistent, I don't think we
> should be afraid of losing robustness in the case of complete removal
> of existing checks for null. Again, I'm happy to keep them, but I'd
> rather throw a standard NPE in this case.
> 
> Sébastien
> 
>>
>> Gilles
>>
>> P.S. Is there an occurrence in CM, where a method can be passed a null
>>      argument?
>>
>>
>>>
>>> Phil
>>>
>>>>
>>>> The second option cares for all the various positions _except_ the
>>>> singly-rooted hierarchy.
>>>>
>>>>
>>>> Regards,
>>>> Gilles
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/9/12 11:11 PM, Sébastien Brisard wrote:
> Hi,
>
> 2012/9/10 Gilles Sadowski <gi...@harfang.homelinux.org>:
>> On Sun, Sep 09, 2012 at 09:16:51AM -0700, Phil Steitz wrote:
>>> On 9/9/12 4:34 AM, Gilles Sadowski wrote:
>>>> Hi.
>>>>
>>>> Further discussion on the JIRA page
>>>>   https://issues.apache.org/jira/browse/MATH-856
>>>> cannot reach a consensus on solving the issue that raised this thread.
>>>>
>>>> The proposal was that CM never checks for "null" and lets the JVM do it (and
>>>> thus throw the standard NPE).
>>>>
>>>> Phil wants to retain some null checks but opposes to throwing a NPE without
>>>> a "detailed message".
>>>> The localization mechanism being implemented in "ExceptionContext", we
>>>> cannot throw a standard NPE (since the error message won't be localized).
>>>>
>>>> For a consistent behaviour (as seen from the caller), we would have to
>>>> implement a subclass of the standard NPE: callers could do
>>>>
>>>>  try {
>>>>    // Call CM
>>>>  } catch (NullPointerException e) {
>>>>    // Handle NPE (raised by the JVM _or_ by CM).
>>>>  }
>>>>
>>>> However, this breaks the consensus we arrived at (for v4.0) about CM
>>>> throwing only subclasses of "MathRuntimeExceprion" (singly rooted hierarchy).
>>>>
>>>> Phil proposes to throw MathIAE (IMO, it must be the specific
>>>> "NullArgumentException"), but this breaks the above use-case: Users have to
>>>> do
>>>>
>>>>  try {
>>>>    // Call CM
>>>>  } catch (NullPointerException e) {
>>>>    // Handle NPE (raised by the JVM).
>>>>  } catch (NullArgumentException e)
>>>>    // Handle NPE (raised by CM).
>>>>  }
>>>>
>>>> showing blatantly that CM is not consistent: sometimes it lets a JVM
>>>> exception propagate, and sometimes it catches the problem eralier and throws
>>>> an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0,
>>>> "MathRuntimeException").
>>>> This is the current state of affairs, and I think that it is not
>>>> satisfactory. [As proven by this issue having recurred two or three times
>>>> already.]
>>>>
>>>> In light of this, I propose that either
>>>> * Phil changes his mind (no check for null performed in CM code), or
>>>> * we make an exception to the singly-rooted hierarchy just for
>>>>   "NullArgumentException" (which, in 4.0, would become a subclass of the
>>>>   standard NPE).
>>> Why not just leave things alone [...]
>> For the reason I gave above: the inconsistent/non-existent policy will make
>> the issue recur sooner or later, as it happened now with Sébastien, as it
>> happened with me when I first signalled the burden of checked excpetions and
>> later when we agreed about MathRuntimeException, then changed again, to come
>> back again now, to where we were almost two years ago (IIRC)...
>>
>>> - i.e., let some APIs document null
>>> handling and throw IAE at the point of method invocation when
>>> supplying a null violates the documented API contract?
>> The answer to that question is in the previous post.
>>
>>> We can leave the (needless, IMO) NullArgumentException as a subclass
>>> of MathIAE in place, or drop it and throw MathIAE directly in these
>>> cases.
>> "NullArgumentException" is about as needless as any subclass of "Exception"
>> or "RuntimeException". Either we use inheritance for what it's primarily
>> meant or we choose another, non-OO, language.
>>
>> This is going in circles; some people will drop from the discussion (or
>> already did) and some time from now, someone will "rediscover" this, among
>> other little defects of CM. These are worth correcting, but not worth
>> discussing endlessly.
>>
>> Let's just have all people here provide their preference and be done with
>> it.
>>
> Since there is no way to enforce a strict policy on checking for null
> in CM, I think that NAE is useless, and should be droped.

I agree there, as long as APIs that specify null-handling and
disallow nulls can throw MathIAE in its place.
> If we assume that every user of CM can use a debugger, then probably
> (and contrary to what I advocated earlier), checking early for null is
> also superfluous.

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

Phil
> I tend to document arguments which *can* be null (I think someone else
> also mentioned this practice), so that it's fairly safe (as someone
> already wrote) to assume that all other arguments *must* be non-null.
>
> To sum up, I would favor complete removal of NAE. As for existing
> checks, I would either remove them, or throw an argumentless standard
> NPE.
>
> Phil was talking about loss of robustness. I don't think that CM as a
> whole is robust with respect to null pointers. In some places, the
> code fails in the standard way (NPE), while in others, it fails in a
> fully documented way. Since this is inconsistent, I don't think we
> should be afraid of losing robustness in the case of complete removal
> of existing checks for null. Again, I'm happy to keep them, but I'd
> rather throw a standard NPE in this case.
>
> Sébastien
>
>> Gilles
>>
>> P.S. Is there an occurrence in CM, where a method can be passed a null
>>      argument?
>>
>>
>>> Phil
>>>
>>>> The second option cares for all the various positions _except_ the
>>>> singly-rooted hierarchy.
>>>>
>>>>
>>>> Regards,
>>>> Gilles
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Sébastien Brisard <se...@m4x.org>.
Hi,

2012/9/10 Gilles Sadowski <gi...@harfang.homelinux.org>:
> On Sun, Sep 09, 2012 at 09:16:51AM -0700, Phil Steitz wrote:
>> On 9/9/12 4:34 AM, Gilles Sadowski wrote:
>> > Hi.
>> >
>> > Further discussion on the JIRA page
>> >   https://issues.apache.org/jira/browse/MATH-856
>> > cannot reach a consensus on solving the issue that raised this thread.
>> >
>> > The proposal was that CM never checks for "null" and lets the JVM do it (and
>> > thus throw the standard NPE).
>> >
>> > Phil wants to retain some null checks but opposes to throwing a NPE without
>> > a "detailed message".
>> > The localization mechanism being implemented in "ExceptionContext", we
>> > cannot throw a standard NPE (since the error message won't be localized).
>> >
>> > For a consistent behaviour (as seen from the caller), we would have to
>> > implement a subclass of the standard NPE: callers could do
>> >
>> >  try {
>> >    // Call CM
>> >  } catch (NullPointerException e) {
>> >    // Handle NPE (raised by the JVM _or_ by CM).
>> >  }
>> >
>> > However, this breaks the consensus we arrived at (for v4.0) about CM
>> > throwing only subclasses of "MathRuntimeExceprion" (singly rooted hierarchy).
>> >
>> > Phil proposes to throw MathIAE (IMO, it must be the specific
>> > "NullArgumentException"), but this breaks the above use-case: Users have to
>> > do
>> >
>> >  try {
>> >    // Call CM
>> >  } catch (NullPointerException e) {
>> >    // Handle NPE (raised by the JVM).
>> >  } catch (NullArgumentException e)
>> >    // Handle NPE (raised by CM).
>> >  }
>> >
>> > showing blatantly that CM is not consistent: sometimes it lets a JVM
>> > exception propagate, and sometimes it catches the problem eralier and throws
>> > an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0,
>> > "MathRuntimeException").
>> > This is the current state of affairs, and I think that it is not
>> > satisfactory. [As proven by this issue having recurred two or three times
>> > already.]
>> >
>> > In light of this, I propose that either
>> > * Phil changes his mind (no check for null performed in CM code), or
>> > * we make an exception to the singly-rooted hierarchy just for
>> >   "NullArgumentException" (which, in 4.0, would become a subclass of the
>> >   standard NPE).
>>
>> Why not just leave things alone [...]
>
> For the reason I gave above: the inconsistent/non-existent policy will make
> the issue recur sooner or later, as it happened now with Sébastien, as it
> happened with me when I first signalled the burden of checked excpetions and
> later when we agreed about MathRuntimeException, then changed again, to come
> back again now, to where we were almost two years ago (IIRC)...
>
>> - i.e., let some APIs document null
>> handling and throw IAE at the point of method invocation when
>> supplying a null violates the documented API contract?
>
> The answer to that question is in the previous post.
>
>> We can leave the (needless, IMO) NullArgumentException as a subclass
>> of MathIAE in place, or drop it and throw MathIAE directly in these
>> cases.
>
> "NullArgumentException" is about as needless as any subclass of "Exception"
> or "RuntimeException". Either we use inheritance for what it's primarily
> meant or we choose another, non-OO, language.
>
> This is going in circles; some people will drop from the discussion (or
> already did) and some time from now, someone will "rediscover" this, among
> other little defects of CM. These are worth correcting, but not worth
> discussing endlessly.
>
> Let's just have all people here provide their preference and be done with
> it.
>
Since there is no way to enforce a strict policy on checking for null
in CM, I think that NAE is useless, and should be droped.
If we assume that every user of CM can use a debugger, then probably
(and contrary to what I advocated earlier), checking early for null is
also superfluous.
I tend to document arguments which *can* be null (I think someone else
also mentioned this practice), so that it's fairly safe (as someone
already wrote) to assume that all other arguments *must* be non-null.

To sum up, I would favor complete removal of NAE. As for existing
checks, I would either remove them, or throw an argumentless standard
NPE.

Phil was talking about loss of robustness. I don't think that CM as a
whole is robust with respect to null pointers. In some places, the
code fails in the standard way (NPE), while in others, it fails in a
fully documented way. Since this is inconsistent, I don't think we
should be afraid of losing robustness in the case of complete removal
of existing checks for null. Again, I'm happy to keep them, but I'd
rather throw a standard NPE in this case.

Sébastien

>
> Gilles
>
> P.S. Is there an occurrence in CM, where a method can be passed a null
>      argument?
>
>
>>
>> Phil
>>
>> >
>> > The second option cares for all the various positions _except_ the
>> > singly-rooted hierarchy.
>> >
>> >
>> > Regards,
>> > Gilles
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > For additional commands, e-mail: dev-help@commons.apache.org
>> >
>> >
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Sun, Sep 09, 2012 at 09:16:51AM -0700, Phil Steitz wrote:
> On 9/9/12 4:34 AM, Gilles Sadowski wrote:
> > Hi.
> >
> > Further discussion on the JIRA page
> >   https://issues.apache.org/jira/browse/MATH-856
> > cannot reach a consensus on solving the issue that raised this thread.
> >
> > The proposal was that CM never checks for "null" and lets the JVM do it (and
> > thus throw the standard NPE).
> >
> > Phil wants to retain some null checks but opposes to throwing a NPE without
> > a "detailed message".
> > The localization mechanism being implemented in "ExceptionContext", we
> > cannot throw a standard NPE (since the error message won't be localized).
> >
> > For a consistent behaviour (as seen from the caller), we would have to
> > implement a subclass of the standard NPE: callers could do
> >
> >  try {
> >    // Call CM
> >  } catch (NullPointerException e) {
> >    // Handle NPE (raised by the JVM _or_ by CM).
> >  }
> >
> > However, this breaks the consensus we arrived at (for v4.0) about CM
> > throwing only subclasses of "MathRuntimeExceprion" (singly rooted hierarchy).
> >
> > Phil proposes to throw MathIAE (IMO, it must be the specific
> > "NullArgumentException"), but this breaks the above use-case: Users have to
> > do
> >
> >  try {
> >    // Call CM
> >  } catch (NullPointerException e) {
> >    // Handle NPE (raised by the JVM).
> >  } catch (NullArgumentException e)
> >    // Handle NPE (raised by CM).
> >  }
> >
> > showing blatantly that CM is not consistent: sometimes it lets a JVM
> > exception propagate, and sometimes it catches the problem eralier and throws
> > an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0,
> > "MathRuntimeException").
> > This is the current state of affairs, and I think that it is not
> > satisfactory. [As proven by this issue having recurred two or three times
> > already.]
> >
> > In light of this, I propose that either
> > * Phil changes his mind (no check for null performed in CM code), or
> > * we make an exception to the singly-rooted hierarchy just for
> >   "NullArgumentException" (which, in 4.0, would become a subclass of the
> >   standard NPE).
> 
> Why not just leave things alone [...]

For the reason I gave above: the inconsistent/non-existent policy will make
the issue recur sooner or later, as it happened now with Sébastien, as it
happened with me when I first signalled the burden of checked excpetions and
later when we agreed about MathRuntimeException, then changed again, to come
back again now, to where we were almost two years ago (IIRC)...

> - i.e., let some APIs document null
> handling and throw IAE at the point of method invocation when
> supplying a null violates the documented API contract?

The answer to that question is in the previous post.

> We can leave the (needless, IMO) NullArgumentException as a subclass
> of MathIAE in place, or drop it and throw MathIAE directly in these
> cases.

"NullArgumentException" is about as needless as any subclass of "Exception"
or "RuntimeException". Either we use inheritance for what it's primarily
meant or we choose another, non-OO, language.

This is going in circles; some people will drop from the discussion (or
already did) and some time from now, someone will "rediscover" this, among
other little defects of CM. These are worth correcting, but not worth
discussing endlessly.

Let's just have all people here provide their preference and be done with
it.


Gilles

P.S. Is there an occurrence in CM, where a method can be passed a null
     argument?


> 
> Phil
> 
> >
> > The second option cares for all the various positions _except_ the
> > singly-rooted hierarchy.
> >
> >
> > Regards,
> > Gilles
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/9/12 4:34 AM, Gilles Sadowski wrote:
> Hi.
>
> Further discussion on the JIRA page
>   https://issues.apache.org/jira/browse/MATH-856
> cannot reach a consensus on solving the issue that raised this thread.
>
> The proposal was that CM never checks for "null" and lets the JVM do it (and
> thus throw the standard NPE).
>
> Phil wants to retain some null checks but opposes to throwing a NPE without
> a "detailed message".
> The localization mechanism being implemented in "ExceptionContext", we
> cannot throw a standard NPE (since the error message won't be localized).
>
> For a consistent behaviour (as seen from the caller), we would have to
> implement a subclass of the standard NPE: callers could do
>
>  try {
>    // Call CM
>  } catch (NullPointerException e) {
>    // Handle NPE (raised by the JVM _or_ by CM).
>  }
>
> However, this breaks the consensus we arrived at (for v4.0) about CM
> throwing only subclasses of "MathRuntimeExceprion" (singly rooted hierarchy).
>
> Phil proposes to throw MathIAE (IMO, it must be the specific
> "NullArgumentException"), but this breaks the above use-case: Users have to
> do
>
>  try {
>    // Call CM
>  } catch (NullPointerException e) {
>    // Handle NPE (raised by the JVM).
>  } catch (NullArgumentException e)
>    // Handle NPE (raised by CM).
>  }
>
> showing blatantly that CM is not consistent: sometimes it lets a JVM
> exception propagate, and sometimes it catches the problem eralier and throws
> an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0,
> "MathRuntimeException").
> This is the current state of affairs, and I think that it is not
> satisfactory. [As proven by this issue having recurred two or three times
> already.]
>
> In light of this, I propose that either
> * Phil changes his mind (no check for null performed in CM code), or
> * we make an exception to the singly-rooted hierarchy just for
>   "NullArgumentException" (which, in 4.0, would become a subclass of the
>   standard NPE).

Why not just leave things alone - i.e., let some APIs document null
handling and throw IAE at the point of method invocation when
supplying a null violates the documented API contract?
We can leave the (needless, IMO) NullArgumentException as a subclass
of MathIAE in place, or drop it and throw MathIAE directly in these
cases.

Phil

>
> The second option cares for all the various positions _except_ the
> singly-rooted hierarchy.
>
>
> Regards,
> Gilles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.

Further discussion on the JIRA page
  https://issues.apache.org/jira/browse/MATH-856
cannot reach a consensus on solving the issue that raised this thread.

The proposal was that CM never checks for "null" and lets the JVM do it (and
thus throw the standard NPE).

Phil wants to retain some null checks but opposes to throwing a NPE without
a "detailed message".
The localization mechanism being implemented in "ExceptionContext", we
cannot throw a standard NPE (since the error message won't be localized).

For a consistent behaviour (as seen from the caller), we would have to
implement a subclass of the standard NPE: callers could do

 try {
   // Call CM
 } catch (NullPointerException e) {
   // Handle NPE (raised by the JVM _or_ by CM).
 }

However, this breaks the consensus we arrived at (for v4.0) about CM
throwing only subclasses of "MathRuntimeExceprion" (singly rooted hierarchy).

Phil proposes to throw MathIAE (IMO, it must be the specific
"NullArgumentException"), but this breaks the above use-case: Users have to
do

 try {
   // Call CM
 } catch (NullPointerException e) {
   // Handle NPE (raised by the JVM).
 } catch (NullArgumentException e)
   // Handle NPE (raised by CM).
 }

showing blatantly that CM is not consistent: sometimes it lets a JVM
exception propagate, and sometimes it catches the problem eralier and throws
an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0,
"MathRuntimeException").
This is the current state of affairs, and I think that it is not
satisfactory. [As proven by this issue having recurred two or three times
already.]

In light of this, I propose that either
* Phil changes his mind (no check for null performed in CM code), or
* we make an exception to the singly-rooted hierarchy just for
  "NullArgumentException" (which, in 4.0, would become a subclass of the
  standard NPE).

The second option cares for all the various positions _except_ the
singly-rooted hierarchy.


Regards,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException"

Posted by Phil Steitz <ph...@gmail.com>.
On 9/6/12 5:16 AM, Gilles Sadowski wrote:
> Hi.
>
>>>>>> [...]
>>>>>>
>>>> I was about to start a new thread too, but refrained to do so for lack
>>>> of knowledge on the history of this particular exception.
>>>> Check for null is unevenly enforced througout the library, which --to
>>>> me-- suggests we shouldn't do it at all and contend with NPE. There is
>>>> one potential use, though. I think we should check for null when the
>>>> NPE might occur in a different method.
>>>>
>>>> This is what happens with new Incrementor(int,
>>>> MaxCountExceededCallback) : cb is just copied, and fields of cb are
>>>> invoked elsewhere: in that case, checking for null does make sense.
>>>
>>> I think even there we could rely on the JVM, for simplicity.
>>>
>> Do you mean that we should do nothing, and let NPE occur "naturally"?
> I'd think we should opt for this.
>
>> The origin of the problem might then be difficult to identify.
> That's true. But the reason of the problem is an "obvious" bug
> (uninitialized variable).
> The exception may be raised further down the call sequence, but the stack
> trace will provide a one way road to discover the source of the problem.
>
>> Or maybe you meant that we check for null in that case, but throw NPE
>> instead of our own MathNullArgument?
> No, it sure is simpler to do nothing in CM. :-)
> Otherwise, we'll have to check every reference (for consistency).
>
>> I would be in favor of the second option. On the other hand it's
>> difficult to check that it's consistently applied throughout our code,
>> and I can see your point in doing nothing.
> So, do we agree?

I would rather fully document what the API does on nulls (which we
do in a lot of places now), but can live with the "go boom" for this
in some (most?) places if that is the consensus of others.  I would
prefer at least not to rip out the null checks that we have in a lot
of places in the stats package.  As a user, I would rather get an
IAE with informative message when I pass a not allowed null than an
untrapped NPE from somewhere a few layers down in the stack.  I
would like to preserve the way the code that does null checks today
works.  Other than that, I am OK dropping the custom [math] exception.

Phil
>
> Gilles
>
>> Sébastien
>>
>>
>> ---------------------------------------------------------------------
>> 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] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.

> >>>> [...]
> >>>>
> >> I was about to start a new thread too, but refrained to do so for lack
> >> of knowledge on the history of this particular exception.
> >> Check for null is unevenly enforced througout the library, which --to
> >> me-- suggests we shouldn't do it at all and contend with NPE. There is
> >> one potential use, though. I think we should check for null when the
> >> NPE might occur in a different method.
> >>
> >> This is what happens with new Incrementor(int,
> >> MaxCountExceededCallback) : cb is just copied, and fields of cb are
> >> invoked elsewhere: in that case, checking for null does make sense.
> >
> >
> > I think even there we could rely on the JVM, for simplicity.
> >
> Do you mean that we should do nothing, and let NPE occur "naturally"?

I'd think we should opt for this.

> The origin of the problem might then be difficult to identify.

That's true. But the reason of the problem is an "obvious" bug
(uninitialized variable).
The exception may be raised further down the call sequence, but the stack
trace will provide a one way road to discover the source of the problem.

> Or maybe you meant that we check for null in that case, but throw NPE
> instead of our own MathNullArgument?

No, it sure is simpler to do nothing in CM. :-)
Otherwise, we'll have to check every reference (for consistency).

> I would be in favor of the second option. On the other hand it's
> difficult to check that it's consistently applied throughout our code,
> and I can see your point in doing nothing.

So, do we agree?

Gilles

> 
> Sébastien
> 
> 
> ---------------------------------------------------------------------
> 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] About "NullArgumentException"

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> > [...]
> >>
> >Do you mean that we should do nothing, and let NPE occur "naturally"?
> >The origin of the problem might then be difficult to identify.
> >Or maybe you meant that we check for null in that case, but throw NPE
> >instead of our own MathNullArgument?
> >I would be in favor of the second option. On the other hand it's
> >difficult to check that it's consistently applied throughout our
> >code,
> >and I can see your point in doing nothing.
> 
> I meant to do nothing, and even to not document that
> NullPointerException can
> occur. There are simply too many places when users provide objects
> we use later on,
> checking everything would be impossible to maintain.
> 
> There are some cases when passing a null reference is allowed, but
> they are the
> minority. They are so rare that I tend to document these specific
> cases only in
> the Javadoc, adding a parenthesis "(can be null)" at the end of the
> @param. So when
> nothing is said, it implicitly means to me "(cannot be null)" and if
> I pass null,
> I should not be surprised if something bad happens.

+1
[Also to be added in the formatting guidelines.]


Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException" (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)

Posted by luc <lu...@spaceroots.org>.
Le 2012-09-06 13:58, Sébastien Brisard a écrit :
> Hi Luc,
>
> 2012/9/6 luc <lu...@spaceroots.org>:
>> Hi all,
>>
>> Le 2012-09-06 07:08, Sébastien Brisard a écrit :
>>
>>> Hi Gilles,
>>>
>>> 2012/9/6 sebb <se...@gmail.com>:
>>>>
>>>> On 5 September 2012 22:46, Gilles Sadowski 
>>>> <gi...@harfang.homelinux.org>
>>>> wrote:
>>>>>
>>>>> On Wed, Sep 05, 2012 at 06:30:08PM -0000, luc@apache.org wrote:
>>>>>>
>>>>>> Author: luc
>>>>>> Date: Wed Sep  5 18:30:08 2012
>>>>>> New Revision: 1381284
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1381284&view=rev
>>>>>> Log:
>>>>>> Added throw declarations for package complex.
>>>>>>
>>>>>> Modified:
>>>>>>
>>>>>> 
>>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>>>>>
>>>>>> 
>>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java
>>>>>>
>>>>>> 
>>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java
>>>>>>
>>>>>> Modified:
>>>>>> 
>>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>>>>> URL:
>>>>>> 
>>>>>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284&r1=1381283&r2=1381284&view=diff
>>>>>>
>>>>>>
>>>>>> 
>>>>>> ==============================================================================
>>>>>> ---
>>>>>> 
>>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>>>>> (original)
>>>>>> +++
>>>>>> 
>>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>>>>> Wed Sep  5 18:30:08 2012
>>>>>> @@ -22,6 +22,7 @@ import java.util.ArrayList;
>>>>>>  import java.util.List;
>>>>>>
>>>>>>  import org.apache.commons.math3.FieldElement;
>>>>>> +import org.apache.commons.math3.exception.MathInternalError;
>>>>>>  import 
>>>>>> org.apache.commons.math3.exception.NullArgumentException;
>>>>>>  import org.apache.commons.math3.exception.NotPositiveException;
>>>>>>  import 
>>>>>> org.apache.commons.math3.exception.util.LocalizedFormats;
>>>>>> @@ -566,12 +567,17 @@ public class Complex implements FieldEle
>>>>>>       * @since 1.2
>>>>>>       */
>>>>>>      public Complex acos() {
>>>>>> -        if (isNaN) {
>>>>>> -            return NaN;
>>>>>> -        }
>>>>>> +        try {
>>>>>> +            if (isNaN) {
>>>>>> +                return NaN;
>>>>>> +            }
>>
>>
>> You are right. It's the result of a bad copy-paste.
>>
>>
>>>>
>>>> Regardless of whether it makes sense to catch NUA the above 
>>>> condition
>>>> should not be part of the try clause.
>>>>
>>>>>>
>>>>>> -        return this.add(this.sqrt1z().multiply(I)).log()
>>>>>> -            .multiply(I.negate());
>>>>>> +            return this.add(this.sqrt1z().multiply(I)).log()
>>>>>> +                    .multiply(I.negate());
>>>>>> +        } catch (NullArgumentException e) {
>>>>>> +            // this should never happen as intermediat results 
>>>>>> are not
>>>>>> null
>>>>>> +            throw new MathInternalError(e);
>>>>>> +        }
>>>>>>      }
>>>>>
>>>>>
>>>>> Maybe I don't understand the purpose of catching 
>>>>> "NullArgumentException"
>>>>> and
>>>>> rethrowing something else.
>>
>>
>> I agree and did not really liked this. It's clearly a false move by 
>> me,
>> sorryr for that.
>>
>>
>>>>>
>>>>> Anyway, I was going to start a new thread about 
>>>>> "NullArgumentException":
>>>>> my
>>>>> proposal is to never check for null and let the standard NPE be 
>>>>> thrown
>>>>> in
>>>>> case of bad usage (null passed where a non-null is required).
>>>>>
>>>>> That would avoid such catch and rethrow for things that never 
>>>>> happen.
>>
>>
>> I think it would be better. Our own NullArgumentException don't 
>> brings any
>> added value
>> and I'll prefer to let the JVM handle this by itself.
>>
>> I don't remember the arguments and positions of everyone when we 
>> discussed
>> it earlier.
>>
>>
>>>>>
>>>>>
>>>>> Best regards,
>>>>> Gilles
>>>>>
>>>>>> [...]
>>>>>
>>>>>
>>> I was about to start a new thread too, but refrained to do so for 
>>> lack
>>> of knowledge on the history of this particular exception.
>>> Check for null is unevenly enforced througout the library, which 
>>> --to
>>> me-- suggests we shouldn't do it at all and contend with NPE. There 
>>> is
>>> one potential use, though. I think we should check for null when 
>>> the
>>> NPE might occur in a different method.
>>>
>>> This is what happens with new Incrementor(int,
>>> MaxCountExceededCallback) : cb is just copied, and fields of cb are
>>> invoked elsewhere: in that case, checking for null does make sense.
>>
>>
>> I think even there we could rely on the JVM, for simplicity.
>>
> Do you mean that we should do nothing, and let NPE occur "naturally"?
> The origin of the problem might then be difficult to identify.
> Or maybe you meant that we check for null in that case, but throw NPE
> instead of our own MathNullArgument?
> I would be in favor of the second option. On the other hand it's
> difficult to check that it's consistently applied throughout our 
> code,
> and I can see your point in doing nothing.

I meant to do nothing, and even to not document that 
NullPointerException can
occur. There are simply too many places when users provide objects we 
use later on,
checking everything would be impossible to maintain.

There are some cases when passing a null reference is allowed, but they 
are the
minority. They are so rare that I tend to document these specific cases 
only in
the Javadoc, adding a parenthesis "(can be null)" at the end of the 
@param. So when
nothing is said, it implicitly means to me "(cannot be null)" and if I 
pass null,
I should not be surprised if something bad happens.

best regards,
Luc

>
> Sébastien
>
>
> ---------------------------------------------------------------------
> 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] About "NullArgumentException" (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)

Posted by Sébastien Brisard <se...@m4x.org>.
Hi Luc,

2012/9/6 luc <lu...@spaceroots.org>:
> Hi all,
>
> Le 2012-09-06 07:08, Sébastien Brisard a écrit :
>
>> Hi Gilles,
>>
>> 2012/9/6 sebb <se...@gmail.com>:
>>>
>>> On 5 September 2012 22:46, Gilles Sadowski <gi...@harfang.homelinux.org>
>>> wrote:
>>>>
>>>> On Wed, Sep 05, 2012 at 06:30:08PM -0000, luc@apache.org wrote:
>>>>>
>>>>> Author: luc
>>>>> Date: Wed Sep  5 18:30:08 2012
>>>>> New Revision: 1381284
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1381284&view=rev
>>>>> Log:
>>>>> Added throw declarations for package complex.
>>>>>
>>>>> Modified:
>>>>>
>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>>>>
>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java
>>>>>
>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java
>>>>>
>>>>> Modified:
>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284&r1=1381283&r2=1381284&view=diff
>>>>>
>>>>>
>>>>> ==============================================================================
>>>>> ---
>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>>>> (original)
>>>>> +++
>>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>>>> Wed Sep  5 18:30:08 2012
>>>>> @@ -22,6 +22,7 @@ import java.util.ArrayList;
>>>>>  import java.util.List;
>>>>>
>>>>>  import org.apache.commons.math3.FieldElement;
>>>>> +import org.apache.commons.math3.exception.MathInternalError;
>>>>>  import org.apache.commons.math3.exception.NullArgumentException;
>>>>>  import org.apache.commons.math3.exception.NotPositiveException;
>>>>>  import org.apache.commons.math3.exception.util.LocalizedFormats;
>>>>> @@ -566,12 +567,17 @@ public class Complex implements FieldEle
>>>>>       * @since 1.2
>>>>>       */
>>>>>      public Complex acos() {
>>>>> -        if (isNaN) {
>>>>> -            return NaN;
>>>>> -        }
>>>>> +        try {
>>>>> +            if (isNaN) {
>>>>> +                return NaN;
>>>>> +            }
>
>
> You are right. It's the result of a bad copy-paste.
>
>
>>>
>>> Regardless of whether it makes sense to catch NUA the above condition
>>> should not be part of the try clause.
>>>
>>>>>
>>>>> -        return this.add(this.sqrt1z().multiply(I)).log()
>>>>> -            .multiply(I.negate());
>>>>> +            return this.add(this.sqrt1z().multiply(I)).log()
>>>>> +                    .multiply(I.negate());
>>>>> +        } catch (NullArgumentException e) {
>>>>> +            // this should never happen as intermediat results are not
>>>>> null
>>>>> +            throw new MathInternalError(e);
>>>>> +        }
>>>>>      }
>>>>
>>>>
>>>> Maybe I don't understand the purpose of catching "NullArgumentException"
>>>> and
>>>> rethrowing something else.
>
>
> I agree and did not really liked this. It's clearly a false move by me,
> sorryr for that.
>
>
>>>>
>>>> Anyway, I was going to start a new thread about "NullArgumentException":
>>>> my
>>>> proposal is to never check for null and let the standard NPE be thrown
>>>> in
>>>> case of bad usage (null passed where a non-null is required).
>>>>
>>>> That would avoid such catch and rethrow for things that never happen.
>
>
> I think it would be better. Our own NullArgumentException don't brings any
> added value
> and I'll prefer to let the JVM handle this by itself.
>
> I don't remember the arguments and positions of everyone when we discussed
> it earlier.
>
>
>>>>
>>>>
>>>> Best regards,
>>>> Gilles
>>>>
>>>>> [...]
>>>>
>>>>
>> I was about to start a new thread too, but refrained to do so for lack
>> of knowledge on the history of this particular exception.
>> Check for null is unevenly enforced througout the library, which --to
>> me-- suggests we shouldn't do it at all and contend with NPE. There is
>> one potential use, though. I think we should check for null when the
>> NPE might occur in a different method.
>>
>> This is what happens with new Incrementor(int,
>> MaxCountExceededCallback) : cb is just copied, and fields of cb are
>> invoked elsewhere: in that case, checking for null does make sense.
>
>
> I think even there we could rely on the JVM, for simplicity.
>
Do you mean that we should do nothing, and let NPE occur "naturally"?
The origin of the problem might then be difficult to identify.
Or maybe you meant that we check for null in that case, but throw NPE
instead of our own MathNullArgument?
I would be in favor of the second option. On the other hand it's
difficult to check that it's consistently applied throughout our code,
and I can see your point in doing nothing.

Sébastien


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException" (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)

Posted by luc <lu...@spaceroots.org>.
Hi all,

Le 2012-09-06 07:08, Sébastien Brisard a écrit :
> Hi Gilles,
>
> 2012/9/6 sebb <se...@gmail.com>:
>> On 5 September 2012 22:46, Gilles Sadowski 
>> <gi...@harfang.homelinux.org> wrote:
>>> On Wed, Sep 05, 2012 at 06:30:08PM -0000, luc@apache.org wrote:
>>>> Author: luc
>>>> Date: Wed Sep  5 18:30:08 2012
>>>> New Revision: 1381284
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1381284&view=rev
>>>> Log:
>>>> Added throw declarations for package complex.
>>>>
>>>> Modified:
>>>>     
>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>>>     
>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java
>>>>     
>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java
>>>>
>>>> Modified: 
>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>>> URL: 
>>>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284&r1=1381283&r2=1381284&view=diff
>>>> 
>>>> ==============================================================================
>>>> --- 
>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java 
>>>> (original)
>>>> +++ 
>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java 
>>>> Wed Sep  5 18:30:08 2012
>>>> @@ -22,6 +22,7 @@ import java.util.ArrayList;
>>>>  import java.util.List;
>>>>
>>>>  import org.apache.commons.math3.FieldElement;
>>>> +import org.apache.commons.math3.exception.MathInternalError;
>>>>  import org.apache.commons.math3.exception.NullArgumentException;
>>>>  import org.apache.commons.math3.exception.NotPositiveException;
>>>>  import org.apache.commons.math3.exception.util.LocalizedFormats;
>>>> @@ -566,12 +567,17 @@ public class Complex implements FieldEle
>>>>       * @since 1.2
>>>>       */
>>>>      public Complex acos() {
>>>> -        if (isNaN) {
>>>> -            return NaN;
>>>> -        }
>>>> +        try {
>>>> +            if (isNaN) {
>>>> +                return NaN;
>>>> +            }

You are right. It's the result of a bad copy-paste.

>>
>> Regardless of whether it makes sense to catch NUA the above 
>> condition
>> should not be part of the try clause.
>>
>>>>
>>>> -        return this.add(this.sqrt1z().multiply(I)).log()
>>>> -            .multiply(I.negate());
>>>> +            return this.add(this.sqrt1z().multiply(I)).log()
>>>> +                    .multiply(I.negate());
>>>> +        } catch (NullArgumentException e) {
>>>> +            // this should never happen as intermediat results 
>>>> are not null
>>>> +            throw new MathInternalError(e);
>>>> +        }
>>>>      }
>>>
>>> Maybe I don't understand the purpose of catching 
>>> "NullArgumentException" and
>>> rethrowing something else.

I agree and did not really liked this. It's clearly a false move by me, 
sorryr for that.

>>>
>>> Anyway, I was going to start a new thread about 
>>> "NullArgumentException": my
>>> proposal is to never check for null and let the standard NPE be 
>>> thrown in
>>> case of bad usage (null passed where a non-null is required).
>>>
>>> That would avoid such catch and rethrow for things that never 
>>> happen.

I think it would be better. Our own NullArgumentException don't brings 
any added value
and I'll prefer to let the JVM handle this by itself.

I don't remember the arguments and positions of everyone when we 
discussed it earlier.

>>>
>>>
>>> Best regards,
>>> Gilles
>>>
>>>> [...]
>>>
> I was about to start a new thread too, but refrained to do so for 
> lack
> of knowledge on the history of this particular exception.
> Check for null is unevenly enforced througout the library, which --to
> me-- suggests we shouldn't do it at all and contend with NPE. There 
> is
> one potential use, though. I think we should check for null when the
> NPE might occur in a different method.
>
> This is what happens with new Incrementor(int,
> MaxCountExceededCallback) : cb is just copied, and fields of cb are
> invoked elsewhere: in that case, checking for null does make sense.

I think even there we could rely on the JVM, for simplicity.

Luc

> Sébastien
>
>
> ---------------------------------------------------------------------
> 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] About "NullArgumentException" (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)

Posted by Sébastien Brisard <se...@m4x.org>.
Hi Gilles,

2012/9/6 sebb <se...@gmail.com>:
> On 5 September 2012 22:46, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
>> On Wed, Sep 05, 2012 at 06:30:08PM -0000, luc@apache.org wrote:
>>> Author: luc
>>> Date: Wed Sep  5 18:30:08 2012
>>> New Revision: 1381284
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1381284&view=rev
>>> Log:
>>> Added throw declarations for package complex.
>>>
>>> Modified:
>>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java
>>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java
>>>
>>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284&r1=1381283&r2=1381284&view=diff
>>> ==============================================================================
>>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java (original)
>>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java Wed Sep  5 18:30:08 2012
>>> @@ -22,6 +22,7 @@ import java.util.ArrayList;
>>>  import java.util.List;
>>>
>>>  import org.apache.commons.math3.FieldElement;
>>> +import org.apache.commons.math3.exception.MathInternalError;
>>>  import org.apache.commons.math3.exception.NullArgumentException;
>>>  import org.apache.commons.math3.exception.NotPositiveException;
>>>  import org.apache.commons.math3.exception.util.LocalizedFormats;
>>> @@ -566,12 +567,17 @@ public class Complex implements FieldEle
>>>       * @since 1.2
>>>       */
>>>      public Complex acos() {
>>> -        if (isNaN) {
>>> -            return NaN;
>>> -        }
>>> +        try {
>>> +            if (isNaN) {
>>> +                return NaN;
>>> +            }
>
> Regardless of whether it makes sense to catch NUA the above condition
> should not be part of the try clause.
>
>>>
>>> -        return this.add(this.sqrt1z().multiply(I)).log()
>>> -            .multiply(I.negate());
>>> +            return this.add(this.sqrt1z().multiply(I)).log()
>>> +                    .multiply(I.negate());
>>> +        } catch (NullArgumentException e) {
>>> +            // this should never happen as intermediat results are not null
>>> +            throw new MathInternalError(e);
>>> +        }
>>>      }
>>
>> Maybe I don't understand the purpose of catching "NullArgumentException" and
>> rethrowing something else.
>>
>> Anyway, I was going to start a new thread about "NullArgumentException": my
>> proposal is to never check for null and let the standard NPE be thrown in
>> case of bad usage (null passed where a non-null is required).
>>
>> That would avoid such catch and rethrow for things that never happen.
>>
>>
>> Best regards,
>> Gilles
>>
>>> [...]
>>
I was about to start a new thread too, but refrained to do so for lack
of knowledge on the history of this particular exception.
Check for null is unevenly enforced througout the library, which --to
me-- suggests we shouldn't do it at all and contend with NPE. There is
one potential use, though. I think we should check for null when the
NPE might occur in a different method.

This is what happens with new Incrementor(int,
MaxCountExceededCallback) : cb is just copied, and fields of cb are
invoked elsewhere: in that case, checking for null does make sense.
Sébastien


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [Math] About "NullArgumentException" (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)

Posted by sebb <se...@gmail.com>.
On 5 September 2012 22:46, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
> On Wed, Sep 05, 2012 at 06:30:08PM -0000, luc@apache.org wrote:
>> Author: luc
>> Date: Wed Sep  5 18:30:08 2012
>> New Revision: 1381284
>>
>> URL: http://svn.apache.org/viewvc?rev=1381284&view=rev
>> Log:
>> Added throw declarations for package complex.
>>
>> Modified:
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java
>>
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284&r1=1381283&r2=1381284&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java (original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java Wed Sep  5 18:30:08 2012
>> @@ -22,6 +22,7 @@ import java.util.ArrayList;
>>  import java.util.List;
>>
>>  import org.apache.commons.math3.FieldElement;
>> +import org.apache.commons.math3.exception.MathInternalError;
>>  import org.apache.commons.math3.exception.NullArgumentException;
>>  import org.apache.commons.math3.exception.NotPositiveException;
>>  import org.apache.commons.math3.exception.util.LocalizedFormats;
>> @@ -566,12 +567,17 @@ public class Complex implements FieldEle
>>       * @since 1.2
>>       */
>>      public Complex acos() {
>> -        if (isNaN) {
>> -            return NaN;
>> -        }
>> +        try {
>> +            if (isNaN) {
>> +                return NaN;
>> +            }

Regardless of whether it makes sense to catch NUA the above condition
should not be part of the try clause.

>>
>> -        return this.add(this.sqrt1z().multiply(I)).log()
>> -            .multiply(I.negate());
>> +            return this.add(this.sqrt1z().multiply(I)).log()
>> +                    .multiply(I.negate());
>> +        } catch (NullArgumentException e) {
>> +            // this should never happen as intermediat results are not null
>> +            throw new MathInternalError(e);
>> +        }
>>      }
>
> Maybe I don't understand the purpose of catching "NullArgumentException" and
> rethrowing something else.
>
> Anyway, I was going to start a new thread about "NullArgumentException": my
> proposal is to never check for null and let the standard NPE be thrown in
> case of bad usage (null passed where a non-null is required).
>
> That would avoid such catch and rethrow for things that never happen.
>
>
> 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


[Math] About "NullArgumentException" (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Wed, Sep 05, 2012 at 06:30:08PM -0000, luc@apache.org wrote:
> Author: luc
> Date: Wed Sep  5 18:30:08 2012
> New Revision: 1381284
> 
> URL: http://svn.apache.org/viewvc?rev=1381284&view=rev
> Log:
> Added throw declarations for package complex.
> 
> Modified:
>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java
>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java
> 
> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java
> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284&r1=1381283&r2=1381284&view=diff
> ==============================================================================
> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java (original)
> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java Wed Sep  5 18:30:08 2012
> @@ -22,6 +22,7 @@ import java.util.ArrayList;
>  import java.util.List;
>  
>  import org.apache.commons.math3.FieldElement;
> +import org.apache.commons.math3.exception.MathInternalError;
>  import org.apache.commons.math3.exception.NullArgumentException;
>  import org.apache.commons.math3.exception.NotPositiveException;
>  import org.apache.commons.math3.exception.util.LocalizedFormats;
> @@ -566,12 +567,17 @@ public class Complex implements FieldEle
>       * @since 1.2
>       */
>      public Complex acos() {
> -        if (isNaN) {
> -            return NaN;
> -        }
> +        try {
> +            if (isNaN) {
> +                return NaN;
> +            }
>  
> -        return this.add(this.sqrt1z().multiply(I)).log()
> -            .multiply(I.negate());
> +            return this.add(this.sqrt1z().multiply(I)).log()
> +                    .multiply(I.negate());
> +        } catch (NullArgumentException e) {
> +            // this should never happen as intermediat results are not null
> +            throw new MathInternalError(e);
> +        }
>      }

Maybe I don't understand the purpose of catching "NullArgumentException" and
rethrowing something else.

Anyway, I was going to start a new thread about "NullArgumentException": my
proposal is to never check for null and let the standard NPE be thrown in
case of bad usage (null passed where a non-null is required).

That would avoid such catch and rethrow for things that never happen.


Best regards,
Gilles

> [...]

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org