commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <>
Subject Re: [math] cleaning code
Date Mon, 28 Mar 2011 01:31:26 GMT
On 3/27/11 9:47 AM, Luc Maisonobe wrote:
> Hi all,
> I have squashed a number of findbugs and checkstyle warnings introduced
> by recent changes (more than one hundred).
Thanks! And sorry...
> There are a few remaining bugs for which I would like some ideas.
> Checkstyle errors:
> In class MannWhitneyUTestImpl, the javadoc for private method
> calculateAsymptoticPValue has a N parameter in the Javadoc which is
> wrong and two n1 and n2 parameters in the signature that are not
> documented. I don't know the exact meaning of n1 and n2, could someone
> fix this javadoc ?
> In class MathUtils, method round(double x, int scale, int
> roundingMethod) we catch RuntimeException to wrap it into
> MathRuntimeException. I think we should not and should simply let the
> RuntimeException go up. What do you think ?
+1 - but lets doc which RTEs will be thrown and why.  Looks to me
like these will come from BigDecimal.setScale() which can throw
ArithmeticException and IllegalArgumentException, both for reasons
that make sense in round(...) activation context.

> Findbugs errors:
> In CMAESOptimizer constructor, we directly store references to the
> inputSigma and boundaries parameters in internal fields, thus exposing
> internal representation. I think we should either clone the arrays or
> document the fact we will reference user arrays and set up a findbug
> exclude filter. What do you think ?
Unless these things are / can be big, we should clone. 
> In CMAESOptimizer DoubleIndex class, there is a compareTo method but no
> equals method (and if we add one, there will be no hashCode method),
> should we add them ?
> SerializablePair extends Pair which is not serializable and does not
> have an accessible void constructor. Should we add such a constructor
> (perhaps setting the two fields to null), should we have
> SerializablePair not extend Pair or are we sure this does work correctly
> and we should add a findbugs excude filter ?
Looks like Gilles fixed this.  Thanks, Gilles!

> best regards,
> Luc
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message