You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gilles Sadowski <gi...@harfang.homelinux.org> on 2011/06/12 22:41:49 UTC
Re: svn commit: r1134939 -
/commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/inference/ChiSquareTestImpl.java
On Sun, Jun 12, 2011 at 03:57:32PM -0000, psteitz@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.
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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1134939 -
/commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/inference/ChiSquareTestImpl.java
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Sun, Jun 12, 2011 at 02:19:11PM -0700, Phil Steitz wrote:
> On 6/12/11 1:41 PM, Gilles Sadowski wrote:
> > On Sun, Jun 12, 2011 at 03:57:32PM -0000, psteitz@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
+1
> 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.
It's the other way around: the initial "cause" is that a number is too
small; that's exactly what is checked by the test
if (in.length < 2)
The context is an additional information which can either be conveyed by
a subclass (my preference) or by using the "ExceptionContext".
The former will make it for even less code since the enum will be hidden in
the exception class (which is why I prefer it).
The ultimate goal is to avoid the numerous duplicates (enum elements with
about the same meaning) in "LocalizedFormats"; this can be achieved by only
using them in exception classes.
> 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.
If the same error appears in more than one class, it could be deemed a
good candidate for defining a new exception class.
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1134939 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/inference/ChiSquareTestImpl.java
Posted by Phil Steitz <ph...@gmail.com>.
On 6/12/11 1:41 PM, Gilles Sadowski wrote:
> On Sun, Jun 12, 2011 at 03:57:32PM -0000, psteitz@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-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