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, 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
