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-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
