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 -, 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. 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
Re: svn commit: r1134939 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/inference/ChiSquareTestImpl.java
On 6/12/11 1:41 PM, Gilles Sadowski wrote: > On Sun, Jun 12, 2011 at 03:57:32PM -, 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, i
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 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 -, 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 +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-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org