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