Hello. On Sat, Jun 23, 2012 at 03:09:15PM -0000, celes...@apache.org wrote: > Author: celestin > Date: Sat Jun 23 15:09:14 2012 > New Revision: 1353140
Either I don't understand this change, or I don't agree with it. > > URL: http://svn.apache.org/viewvc?rev=1353140&view=rev > Log: > In o.a.c.m3.Incrementor, modified constructor to allow for null values of the > MaxCountExceededCallback. Null value was previously not checked wich could > lead to a NullPointerException much later (at exhaustion of the counter). > > Modified: > > commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java > > Modified: > commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java > URL: > http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java?rev=1353140&r1=1353139&r2=1353140&view=diff > ============================================================================== > --- > commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java > (original) > +++ > commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/Incrementor.java > Sat Jun 23 15:09:14 2012 > @@ -58,13 +58,7 @@ public class Incrementor { > * @param max Maximal count. > */ > public Incrementor(int max) { > - this(max, > - new MaxCountExceededCallback() { > - /** {@inheritDoc} */ > - public void trigger(int max) { > - throw new MaxCountExceededException(max); > - } > - }); > + this(max, null); > } You set the callback to "null" here but... > /** > @@ -72,12 +66,22 @@ public class Incrementor { > * counter exhaustion. > * > * @param max Maximal count. > - * @param cb Function to be called when the maximal count has been > reached. > + * @param cb Function to be called when the maximal count has been > reached > + * (can be {@code null}). > */ > public Incrementor(int max, > MaxCountExceededCallback cb) { > maximalCount = max; > - maxCountCallback = cb; > + if (cb != null) { > + maxCountCallback = cb; > + } else { > + maxCountCallback = new MaxCountExceededCallback() { > + /** {@inheritDoc} */ > + public void trigger(int max) { > + throw new MaxCountExceededException(max); > + } > + }; > + } > } ... here, when it *is* null, you set it to something not null. So it seems to be a contorted way of disallowing null while saying the opposite in the Javadoc! The first constructor was fine as it was. In the second, a check for not null was missing: --- public Incrementor(int max, MaxCountExceededCallback cb) { if (cb == null) { throw new NullArgumentException(); } maximalCount = max; maxCountCallback = cb; } --- Best regards, Gilles --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
