On 6/12/11 1:41 PM, Gilles Sadowski wrote:
> On Sun, Jun 12, 2011 at 03:57:32PM -0000, pste...@apache.org wrote:
>> Author: psteitz
>> Date: Sun Jun 12 15:57:32 2011
>> New Revision: 1134939
>>
>> URL: http://svn.apache.org/viewvc?rev=1134939&view=rev
>> Log:
>> Restored specificity in exception error messages.
>>
>> Modified:
>>     
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/inference/ChiSquareTestImpl.java
>>
>> Modified: 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/inference/ChiSquareTestImpl.java
>> URL: 
>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/inference/ChiSquareTestImpl.java?rev=1134939&r1=1134938&r2=1134939&view=diff
>> ==============================================================================
>> --- 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/inference/ChiSquareTestImpl.java
>>  (original)
>> +++ 
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/inference/ChiSquareTestImpl.java
>>  Sun Jun 12 15:57:32 2011
>> @@ -20,7 +20,6 @@ import org.apache.commons.math.MathExcep
>>  import org.apache.commons.math.exception.NotPositiveException;
>>  import org.apache.commons.math.exception.NotStrictlyPositiveException;
>>  import org.apache.commons.math.exception.NullArgumentException;
>> -import org.apache.commons.math.exception.NumberIsTooSmallException;
>>  import org.apache.commons.math.exception.OutOfRangeException;
>>  import org.apache.commons.math.exception.DimensionMismatchException;
>>  import org.apache.commons.math.exception.MathIllegalArgumentException;
>> @@ -321,11 +320,13 @@ public class ChiSquareTestImpl implement
>>       */
>>      private void checkArray(long[][] in) {
>>          if (in.length < 2) {
>> -            throw new NumberIsTooSmallException(in.length, 2, true);
>> +            throw new MathIllegalArgumentException(
>> +                    LocalizedFormats.INSUFFICIENT_DIMENSION, in.length, 2);
>>          }
> It's preferable to keep a specific exception type when one exists.
> If a contextual message is necessary, it can be added through the
> "ExceptionContext":
>
> E.g. for the above code excerpt:
> ---CUT---
>     private void checkArray(long[][] in) {
>       final int len = in.length;
>         final int minLen = 2;
>         if (len < minLen) {
>             MathIllegalArgumentException err = new 
> NumberIsTooSmallException(len, minLen, true);
>             
> err.getContext().addMessage(LocalizedFormats.INSUFFICIENT_DIMENSION, len, 
> minLen);
>             throw err;
>         }
>     }
> ---CUT---
>
> And similarly for all the changes in this commit.

In this case, we either need to define new specific exceptions or
(my preference) leave as is.  Why add all of the code above to add
no additional meaning to the IllegalArgumentException?  The problem
in the first case is that (as stated in the exception message) the
input data array has insufficient dimensionality.  Calling it a
"NumberIsTooSmallException" adds nothing.  In the second case, the
expected counts array needs to be fully non-negative, and the
exception needs to report which element violates the condition. 
There are several other IllegalArgumentExceptions used elsewhere in
this class.  I see no reason to change them all to
precondition-specific exceptions. 

Phil
>
> Thanks,
> Gilles
>
>>  
>>          if (in[0].length < 2) {
>> -            throw new NumberIsTooSmallException(in[0].length, 2, true);
>> +            throw new MathIllegalArgumentException(
>> +                    LocalizedFormats.INSUFFICIENT_DIMENSION, in[0].length, 
>> 2);
>>          }
>>  
>>          checkRectangular(in);
>> @@ -354,7 +355,7 @@ public class ChiSquareTestImpl implement
>>      }
>>  
>>      /**
>> -     * Check all entries of the input array are strictly postive.
>> +     * Check all entries of the input array are strictly positive.
>>       *
>>       * @param in Array to be tested.
>>       * @exception NotStrictlyPositiveException if one entry is not positive.
>> @@ -362,7 +363,9 @@ public class ChiSquareTestImpl implement
>>      private void checkPositive(double[] in) {
>>          for (int i = 0; i < in.length; i++) {
>>              if (in[i] <= 0) {
>> -                throw new NotStrictlyPositiveException(in[i]);
>> +                throw new MathIllegalArgumentException(
>> +                        LocalizedFormats.NOT_POSITIVE_ELEMENT_AT_INDEX,
>> +                        i, in[i]);
>>              }
>>          }
>>      }
>> @@ -376,7 +379,9 @@ public class ChiSquareTestImpl implement
>>      private void checkNonNegative(long[] in) {
>>          for (int i = 0; i < in.length; i++) {
>>              if (in[i] < 0) {
>> -                throw new NotPositiveException(in[i]);
>> +                throw new MathIllegalArgumentException(
>> +                        LocalizedFormats.NEGATIVE_ELEMENT_AT_INDEX,
>> +                        i, in[i]);
>>              }
>>          }
>>      }
>> @@ -391,7 +396,9 @@ public class ChiSquareTestImpl implement
>>          for (int i = 0; i < in.length; i ++) {
>>              for (int j = 0; j < in[i].length; j++) {
>>                  if (in[i][j] < 0) {
>> -                    throw new NotPositiveException(in[i][j]);
>> +                    throw new MathIllegalArgumentException(
>> +                            LocalizedFormats.NEGATIVE_ELEMENT_AT_2D_INDEX,
>> +                            i, j, in[i][j]);
>>                  }
>>              }
>>          }
>>
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>


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

Reply via email to