commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: [math] cleaning code
Date Mon, 28 Mar 2011 05:25:17 GMT
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.
>> 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


Mime
View raw message