commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Luc Maisonobe <Luc.Maison...@free.fr>
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 ?

Luc

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message