commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Luc Maisonobe <>
Subject Re: [math] cleaning code
Date Mon, 28 Mar 2011 18:14:47 GMT
Le 28/03/2011 07:25, Phil Steitz a écrit :
> On 3/27/11 6:31 PM, Phil Steitz wrote:
>> 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.
> I will take care of this one.
>>> 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. 
> I can also do this, if others agree.

I also think it would improve robustness. What do the CMAES optimizer
specialists think ?


>>> 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 ?
>> Yes.
> And this.
> Phil
>>> 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!
>> Phil
>>> best regards,
>>> Luc
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail:
>>> For additional commands, e-mail:
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:

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

View raw message