This is a side note.  In the class Incrementor there's a 
MaxCountExceededCallback that triggers the MaxCountExceededException.  It might 
make sense to place the code that throws the exception in a static utility 
method inside the exception, eliminating the cb property, the 
MaxCountExceededCallback interface, and corresponding Incrementor constructor.  
So we could do:

public void incrementCount() throws MaxCountExceededException() {
   MaxCountExceededException.throw(++count, max);
}

Cheers,
Ole




On 08/28/2015 09:33 AM, Gilles wrote:
On Fri, 28 Aug 2015 04:16:09 +0200, Gilles wrote:
On Wed, 26 Aug 2015 23:03:36 +0200, Gilles wrote:
Hi.

On Wed, 26 Aug 2015 18:33:29 +0200, Cahangirova Gunel wrote:
Dear all,

Recently I have reported a bug (in my opinion) MATH-1259 in class
Incrementor from the org.apache.commons.math4.util package, as it is
possible to pass a negative number for the max variable. Given that
the count variable is initialised to 0 by default, it means that
canIncrement() method will always return false.
I wondered whether it is an acceptable behaviour, given that
documentation says that Incrementor is a utility that increments a
counter until a maximum is reached. And in the case of a negative max
this maximum is not reachable at all.

The reply by Gilles was that initially the concept of a class was of
a "counter" rather than an “incrementor”, so one suggested change
might be to make the class an incrementor by specifying initial and
final values, rather than just count number, and the other suggested
change is to just forbid negative counts by throwing an exception.

The question of which change to apply needs a further discussion.

I propose to add the "incrementor" functionality in 4.0, together with
the update proposed in MATH-913.

Actually, I think I'll resolve MATH-913 as "won't fix" (unless there is
an identified use for "long" in math algorithms).

A new contructor will be defined:
---CUT---
  public Incrementor(long max, long start) { /* ... */ }
---CUT---
[Or should it rather be "Incrementor(long start, long max)"?]

Please have a look the new class attached to the issue's JIRA page:
  https://issues.apache.org/jira/browse/MATH-1259

Any objection to the proposed fix (and extension)?


To be noted: this will require modifications of classes that use the
current (to-be-deprecated) "Incrementor".
In line with the "fluent API", the "setMaximalCount" method has been
removed from the new implementation: a new incrementor must be created
instead, thus preventing a "final" incrementor instance.
This should not be a real problem in the sense that such a class was
not thread-safe anyway (because of the variable "count" inside the
used "Incrementor").  [Now it will be explicit from looking at the
using class.]


Gilles


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

Reply via email to