You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Cahangirova Gunel <gu...@gmail.com> on 2015/08/26 18:33:29 UTC

[math] Negative max in class Incrementor

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.

Best regards,
Gunel Jahangirova.


Re: [math] Negative max in class Incrementor

Posted by Ole Ersoy <ol...@gmail.com>.

On 08/30/2015 04:13 PM, Gilles wrote:
> On Sun, 30 Aug 2015 14:45:19 -0500, Ole Ersoy wrote:
>> On 08/30/2015 08:18 AM, Gilles wrote:
>>> Hi.
>>>
>>> On Sat, 29 Aug 2015 22:21:55 -0500, Ole Ersoy wrote:
>>>> I'm deleting most of the discussion, because I think I may be
>>>> throwing too many ingredients on the table.  For further context
>>>> please see the previous thread.
>>>>
>>>>> I don't get that.  Here the main purpose is to set a hard limit
>>>>> that will raise an exception (to avoid that some algo from running
>>>>> forever).
>>>> Well there are two concerns here.  One is the precise number of steps
>>>> that should be executed and the other is whether we need to raise the
>>>> exception.
>>>>
>>>> To stop the algorithm from running forever we let the `end` callback
>>>> notify the thing doing the incrementing that we are done. Does that
>>>> make sense?
>>>
>>> Not sure. The current usage is
>>> 1. Set the hard limit (and action when that limit is reached)
>>> 2. Let the incrementor do its thing (count and call "trigger").
>> IIUC there are three ways of using IntegerSequence as is:
>> A)
>> 1) Initialize the incrementor
>> 2) check canIncrement()
>> 3) get the increment
>> 4) check canIncrement()
>> 5) Increment if we can
>> 6) Check again...
>> 7 If we can't increment, were done, so we get the result of whatever
>> and move on.
>>
>> B)
>> We register a calllback and just keep incrementing until the callback
>> is called.
>>
>> C) We catch the MaxCountExceededException
>>
>> We should only do B, because A is wasteful and C is not necessary.
>
> A is not used in CM
Not yet :).  Seriously though developers look at Commons Math as a library made by brilliant people, such as yourself (Sincerely), and when they see something is part of the API, there's a good chance they will use it.  In this case I think that would be a bit tragic.

>
> B and C are. The former in order to signal failure (unexpectedly large
> number of iterations),
In which case I think the machinery for communicating and dealing with that belongs on the Algorithm class, and should be separate from the IntegerSequence.

> the latter in order to change the exception into
> a more meaningful one (catch and rethrow instead of registering another
> callback).
Is there a case where that's necessary, or could it just as easily be dealt with in the CB?  Overall I think commons math, and consumers of it, would be better served if there were two types of CBs.  One that's internal and deals with internal stuff, and one that's for the public API.  So instead of receiving abstract exceptions that have to be mapped to a problem root cause before the application can decide on how to message it, a CB is registered that the application uses for messaging and as a bridge to the precise root cause.

>
>
>>>
>>> IIUC, you propose that the increment tells its caller that the
>>> limit has been reached.
>> The CB tells the caller that the limit has been reached.  If the
>> incrementer extends the callback interface, and registers with the
>> caller, then yes.  But the CB could be completely separate from the
>> Algorithm instance and the incrementer.
>>
>>> Then, I suppose, the caller will have to
>>> act, rather than the incrementor.
>> Case A)
>> Continue until we converge on a solution that's good enough.  We
>> register the 'increment' CB which notifies the algorithm that we are
>> 'good enough', and if we don't converge then the `end` CB is called,
>> and we take whatever action is necessary.
>
> That's a completely different way of handling the evaluation of the
> result.  Currently this is part of the algorithm logic (through
> "tolerance" parameters).

So if we have one way only of handling various possibilities that's easy to use that's a good thing right?  What I like about it is that it affords the designer the ability to communicate exactly what has happened and notify the CB, and the CB API client can take it from there.  For example we could have an Enum that contains a series of values that map to various possibilities (Similar to HTTP status codes), and these could also be used to keys for I18N.

>
>>
>> Case B)
>> We want to exhaust all the steps so we register an 'end' CB.  We want
>> to find the best possible solution in the allowed number of steps (We
>> already now that we will not find the global optimal).
>
> Return whatever was found after some number of steps is dangerous.
By setting a max we are saying that we want to be notified at some point.  Hopefully we handle the notification in a smart way.
> There was a discussion about it in the past, and IIRC the conclusion
> was that it would not be "standard" behaviour.
Maybe it should be?
> In the optimizers, the "ConvergenceChecker" interface can be subverted
> to count iterations/evaluations and accept a solution that does not
> match the convergence criteria.
I would love to be able to register a CB that's specific to each optimizer where the Optimizer communicates the optimal result along with a LevenbergMarquardtResultsEnum.OPTIMAL, or no result with LevenbergMarquardtResultsEnum.COULD_NOT_CONVERGE, or perhaps something else that's well documented and unique.

>
>
> I still don't see why some algo should pass through the incrementor
> "service" (dealing with 3 callbacks) in order to evaluate the current
> state of its work..

My bad on that one.  It should not, unless it should.  I just used an the ability to hook 3 different callbacks as an example of a 'Flexible / exensible' design.  The IntegerSequenceEventEmitter design should emit whatever events the algorithm needs.  The nice thing about the callbacks is that they can be used to break the algorithm up into discrete steps that listen for events that are specific to the stage that the algorithm is in.

>
>>> Unless I'm missing something, I don't see the advantage (in the
>>> examples from CM).
>>
>> The advantages are that we eliminate:
>> - `canIncrement()`
>> -  `hasIncrement()`
>> - incrementNTimes()
>> - MaxCountExceededException
>> - MaxCountExceededException CB boilerplate
>>
>> We gain a simpler API.
>
> ?
> In CM, the throwing of an exception is mandated by the failure to
> stop before counter exhaustion.
Lets unmandate.

>
> The other methods are used within the Incrementor class, to provide
> the "Iterator" and "range" functionality. [Those are not used within
> CM, but I thought that they were interesting...]
If I had not been reading a ton of NodeJS code lately I would think so too, but now that I'm better at functional programming I think they are a bad idea :-(.

>
>>>> Secondly suppose we expect a sequence like 5, 10, 15, 20...but the
>>>> max is 17.  Do we loop past 17 and let that be the final loop, thus
>>>> passing in 20 to the increment listener / cb, or do we stop at 15?
>>>
>>> The proposed IntegerSequence.Incrementor would trigger the callback
>>> if "increment()" is called again after 15 is returned.
>> I think it's better if we just leave it up to the designer to figure
>> out the max number of steps.
>>
>>> Note that the increment does not stop by itself. If a range is created
>>> its last value will be 15 (and the callback will never be called, by
>>> construction).
>> That's another reason why I like just specifying only:
>> - start
>> - stepSize
>> - maxNumberOfSteps.
>>
>> It's very easy to understand.  The callback is called when the
>> `maxNumberOfSteps` is reached  Not that the other way is that
>> complicated either :).
>
> In the basic usage (where the step == 1), it's fairly identical.
Yup
>
> Sometimes a "range" might be more natural.
Something like the IntStream API.

>
>
>> One thing I am saying though that is that if the `maxNumberOfSteps`
>> is reached and no `end` callback is registered, then we can still call
>> increment.  It just results in a noop().  An exception is not thrown.
>> If the caller wants to be notified when the counter is done, they have
>> to register a callback.
>
> What would a use-case for CM?
It gets rid of the exception and `encourages` the client to register the CB if they want the notification.  I prefer the CB for notification, and I think we should have a CB, or an Exception, but not both.  I take it back.  I think we should just have a CB.

>
>
>>>> By
>>>> letting the developer calculate the number of steps, avoiding the use
>>>> of a max, we gain simplicity and flexibility.
>>>
>>> In all CM usage, the number of steps is unknown; the Incrementor is
>>> intended to avoid an infinite loop.
>>
>> IIUC the max number of steps are known?  The number of steps until we
>> converge are unknown...and could reach the max number of steps.
>
> As I said, it is mainly a fool-proof way to indicate failure rather than
> run forever if e.g. the problem uncovered some implementation bug or
> numerical problem (as it happened in a root solver).
A CB will do a better more lightweight (Less lines of code) job of delivering the message and mapping a root cause when necessary.

>
>>>> Lastly perhaps the `increment` callback wants to notify the
>>>> `incrementer / algorithm` that it's done.  In this case we only
>>>> specify the `start` and `stepSize` arguments and leave it up to the
>>>> `increment` callback to realize the termination point and stop the
>>>> algorithm.
>>>
>>> Cf. previous paragraph.
>> I think we are saying the same thing there.
>>>
>>> Termination of the counter is not the same as termination of the
>>> algorithm.
>> I understand.  By using an incrementor and a max we are incrementing
>> either until:
>> - the algorithm concludes
>> - We reach a max and use the result
>
> Dangerous (cf. above).  It's better to relax the convergence criterion
> and be sure that the solution matches the request.
Sure - I won't talk about doing this via a CB as people following this are probably getting a headache, if they have not dropped out years ago :).

>
>> - We reach a max and indicate an error because we could not converge
>> on a result
>>
>> All of these can be handled in a `increment` CB and a `end` CB.
>
> Sure, but it requires a redesign.  IIRC there was a proposal quite some
> time ago to rethink the class hierarchy in terms of an "IterativeAlgorithm".
Glad to hear that it has mindshare.

>
>>>   In CM usage, the latter must occur first, and the second
>>> signals a failure.
>>
>> So when the `end` CB is called we either succeeded or had a failure.
>> The end CB can evaluate the results and determine either one. The
>> algorithm could also signal to the `end` CB whether the result is a
>> failure or success.
>
> To further discuss, we'd have to see a bigger picture of how it would apply
> to e.g. the "o.a.c.m.optim" package.

Cool.  One of these days I'm going to make time for a deep dive. The code is an invaluable learning resource.

Cheers,
- Ole

>
> Regards,
> Gilles
>
>>> P.S. I'll commit the "IntegerSequence" class. You can try and patch it
>>>      (or design an alternative) to show what you mean on some specific
>>>      CM use of the old "Incrementor".
>>
>> OK I think it would be fun to play more with this.  I've been meaning
>> to experiment with JDK 8 asynchronous callbacks as well. Hope to get
>> some free time soon!
>>
>> Cheers,
>> - Ole
>
>
> ---------------------------------------------------------------------
> 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


Re: [math] Negative max in class Incrementor

Posted by Gilles <gi...@harfang.homelinux.org>.
On Sun, 30 Aug 2015 14:45:19 -0500, Ole Ersoy wrote:
> On 08/30/2015 08:18 AM, Gilles wrote:
>> Hi.
>>
>> On Sat, 29 Aug 2015 22:21:55 -0500, Ole Ersoy wrote:
>>> I'm deleting most of the discussion, because I think I may be
>>> throwing too many ingredients on the table.  For further context
>>> please see the previous thread.
>>>
>>>> I don't get that.  Here the main purpose is to set a hard limit
>>>> that will raise an exception (to avoid that some algo from running
>>>> forever).
>>> Well there are two concerns here.  One is the precise number of 
>>> steps
>>> that should be executed and the other is whether we need to raise 
>>> the
>>> exception.
>>>
>>> To stop the algorithm from running forever we let the `end` 
>>> callback
>>> notify the thing doing the incrementing that we are done.  Does 
>>> that
>>> make sense?
>>
>> Not sure. The current usage is
>> 1. Set the hard limit (and action when that limit is reached)
>> 2. Let the incrementor do its thing (count and call "trigger").
> IIUC there are three ways of using IntegerSequence as is:
> A)
> 1) Initialize the incrementor
> 2) check canIncrement()
> 3) get the increment
> 4) check canIncrement()
> 5) Increment if we can
> 6) Check again...
> 7 If we can't increment, were done, so we get the result of whatever
> and move on.
>
> B)
> We register a calllback and just keep incrementing until the callback
> is called.
>
> C) We catch the MaxCountExceededException
>
> We should only do B, because A is wasteful and C is not necessary.

A is not used in CM
B and C are. The former in order to signal failure (unexpectedly large
number of iterations), the latter in order to change the exception into
a more meaningful one (catch and rethrow instead of registering another
callback).

>>
>> IIUC, you propose that the increment tells its caller that the
>> limit has been reached.
> The CB tells the caller that the limit has been reached.  If the
> incrementer extends the callback interface, and registers with the
> caller, then yes.  But the CB could be completely separate from the
> Algorithm instance and the incrementer.
>
>> Then, I suppose, the caller will have to
>> act, rather than the incrementor.
> Case A)
> Continue until we converge on a solution that's good enough.  We
> register the 'increment' CB which notifies the algorithm that we are
> 'good enough', and if we don't converge then the `end` CB is called,
> and we take whatever action is necessary.

That's a completely different way of handling the evaluation of the
result.  Currently this is part of the algorithm logic (through
"tolerance" parameters).

>
> Case B)
> We want to exhaust all the steps so we register an 'end' CB.  We want
> to find the best possible solution in the allowed number of steps (We
> already now that we will not find the global optimal).

Return whatever was found after some number of steps is dangerous.
There was a discussion about it in the past, and IIRC the conclusion
was that it would not be "standard" behaviour.
In the optimizers, the "ConvergenceChecker" interface can be subverted
to count iterations/evaluations and accept a solution that does not
match the convergence criteria.

I still don't see why some algo should pass through the incrementor
"service" (dealing with 3 callbacks) in order to evaluate the current
state of its work..

>> Unless I'm missing something, I don't see the advantage (in the
>> examples from CM).
>
> The advantages are that we eliminate:
> - `canIncrement()`
> -  `hasIncrement()`
> - incrementNTimes()
> - MaxCountExceededException
> - MaxCountExceededException CB boilerplate
>
> We gain a simpler API.

?
In CM, the throwing of an exception is mandated by the failure to
stop before counter exhaustion.

The other methods are used within the Incrementor class, to provide
the "Iterator" and "range" functionality. [Those are not used within
CM, but I thought that they were interesting...]

>>> Secondly suppose we expect a sequence like 5, 10, 15, 20...but the
>>> max is 17.  Do we loop past 17 and let that be the final loop, thus
>>> passing in 20 to the increment listener / cb, or do we stop at 15?
>>
>> The proposed IntegerSequence.Incrementor would trigger the callback
>> if "increment()" is called again after 15 is returned.
> I think it's better if we just leave it up to the designer to figure
> out the max number of steps.
>
>> Note that the increment does not stop by itself. If a range is 
>> created
>> its last value will be 15 (and the callback will never be called, by
>> construction).
> That's another reason why I like just specifying only:
> - start
> - stepSize
> - maxNumberOfSteps.
>
> It's very easy to understand.  The callback is called when the
> `maxNumberOfSteps` is reached  Not that the other way is that
> complicated either :).

In the basic usage (where the step == 1), it's fairly identical.

Sometimes a "range" might be more natural.

> One thing I am saying though that is that if the `maxNumberOfSteps`
> is reached and no `end` callback is registered, then we can still 
> call
> increment.  It just results in a noop().  An exception is not thrown.
> If the caller wants to be notified when the counter is done, they 
> have
> to register a callback.

What would a use-case for CM?

>>> By
>>> letting the developer calculate the number of steps, avoiding the 
>>> use
>>> of a max, we gain simplicity and flexibility.
>>
>> In all CM usage, the number of steps is unknown; the Incrementor is
>> intended to avoid an infinite loop.
>
> IIUC the max number of steps are known?  The number of steps until we
> converge are unknown...and could reach the max number of steps.

As I said, it is mainly a fool-proof way to indicate failure rather 
than
run forever if e.g. the problem uncovered some implementation bug or
numerical problem (as it happened in a root solver).

>>> Lastly perhaps the `increment` callback wants to notify the
>>> `incrementer / algorithm` that it's done.  In this case we only
>>> specify the `start` and `stepSize` arguments and leave it up to the
>>> `increment` callback to realize the termination point and stop the
>>> algorithm.
>>
>> Cf. previous paragraph.
> I think we are saying the same thing there.
>>
>> Termination of the counter is not the same as termination of the
>> algorithm.
> I understand.  By using an incrementor and a max we are incrementing
> either until:
> - the algorithm concludes
> - We reach a max and use the result

Dangerous (cf. above).  It's better to relax the convergence criterion
and be sure that the solution matches the request.

> - We reach a max and indicate an error because we could not converge
> on a result
>
> All of these can be handled in a `increment` CB and a `end` CB.

Sure, but it requires a redesign.  IIRC there was a proposal quite some
time ago to rethink the class hierarchy in terms of an 
"IterativeAlgorithm".

>>   In CM usage, the latter must occur first, and the second
>> signals a failure.
>
> So when the `end` CB is called we either succeeded or had a failure.
> The end CB can evaluate the results and determine either one.  The
> algorithm could also signal to the `end` CB whether the result is a
> failure or success.

To further discuss, we'd have to see a bigger picture of how it would 
apply
to e.g. the "o.a.c.m.optim" package.

Regards,
Gilles

>> P.S. I'll commit the "IntegerSequence" class. You can try and patch 
>> it
>>      (or design an alternative) to show what you mean on some 
>> specific
>>      CM use of the old "Incrementor".
>
> OK I think it would be fun to play more with this.  I've been meaning
> to experiment with JDK 8 asynchronous callbacks as well. Hope to get
> some free time soon!
>
> Cheers,
> - Ole


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [math] Negative max in class Incrementor

Posted by Ole Ersoy <ol...@gmail.com>.

On 08/30/2015 08:18 AM, Gilles wrote:
> Hi.
>
> On Sat, 29 Aug 2015 22:21:55 -0500, Ole Ersoy wrote:
>> I'm deleting most of the discussion, because I think I may be
>> throwing too many ingredients on the table.  For further context
>> please see the previous thread.
>>
>>> I don't get that.  Here the main purpose is to set a hard limit
>>> that will raise an exception (to avoid that some algo from running
>>> forever).
>> Well there are two concerns here.  One is the precise number of steps
>> that should be executed and the other is whether we need to raise the
>> exception.
>>
>> To stop the algorithm from running forever we let the `end` callback
>> notify the thing doing the incrementing that we are done.  Does that
>> make sense?
>
> Not sure. The current usage is
> 1. Set the hard limit (and action when that limit is reached)
> 2. Let the incrementor do its thing (count and call "trigger").
IIUC there are three ways of using IntegerSequence as is:
A)
1) Initialize the incrementor
2) check canIncrement()
3) get the increment
4) check canIncrement()
5) Increment if we can
6) Check again...
7 If we can't increment, were done, so we get the result of whatever and move on.

B)
We register a calllback and just keep incrementing until the callback is called.

C) We catch the MaxCountExceededException

We should only do B, because A is wasteful and C is not necessary.

>
> IIUC, you propose that the increment tells its caller that the
> limit has been reached.
The CB tells the caller that the limit has been reached.  If the incrementer extends the callback interface, and registers with the caller, then yes.  But the CB could be completely separate from the Algorithm instance and the incrementer.

> Then, I suppose, the caller will have to
> act, rather than the incrementor.
Case A)
Continue until we converge on a solution that's good enough.  We register the 'increment' CB which notifies the algorithm that we are 'good enough', and if we don't converge then the `end` CB is called, and we take whatever action is necessary.

Case B)
We want to exhaust all the steps so we register an 'end' CB.  We want to find the best possible solution in the allowed number of steps (We already now that we will not find the global optimal).


> Unless I'm missing something, I don't see the advantage (in the
> examples from CM).

The advantages are that we eliminate:
- `canIncrement()`
-  `hasIncrement()`
- incrementNTimes()
- MaxCountExceededException
- MaxCountExceededException CB boilerplate

We gain a simpler API.

>
>> Secondly suppose we expect a sequence like 5, 10, 15, 20...but the
>> max is 17.  Do we loop past 17 and let that be the final loop, thus
>> passing in 20 to the increment listener / cb, or do we stop at 15?
>
> The proposed IntegerSequence.Incrementor would trigger the callback
> if "increment()" is called again after 15 is returned.
I think it's better if we just leave it up to the designer to figure out the max number of steps.

> Note that the increment does not stop by itself. If a range is created
> its last value will be 15 (and the callback will never be called, by
> construction).
That's another reason why I like just specifying only:
- start
- stepSize
- maxNumberOfSteps.

It's very easy to understand.  The callback is called when the `maxNumberOfSteps` is reached  Not that the other way is that complicated either :).

One thing I am saying though that is that if the `maxNumberOfSteps` is reached and no `end` callback is registered, then we can still call increment.  It just results in a noop().  An exception is not thrown.  If the caller wants to be notified when the counter is done, they have to register a callback.

>
>
>> By
>> letting the developer calculate the number of steps, avoiding the use
>> of a max, we gain simplicity and flexibility.
>
> In all CM usage, the number of steps is unknown; the Incrementor is
> intended to avoid an infinite loop.

IIUC the max number of steps are known?  The number of steps until we converge are unknown...and could reach the max number of steps.

>
>
>> Lastly perhaps the `increment` callback wants to notify the
>> `incrementer / algorithm` that it's done.  In this case we only
>> specify the `start` and `stepSize` arguments and leave it up to the
>> `increment` callback to realize the termination point and stop the
>> algorithm.
>
> Cf. previous paragraph.
I think we are saying the same thing there.
>
> Termination of the counter is not the same as termination of the
> algorithm.
I understand.  By using an incrementor and a max we are incrementing either until:
- the algorithm concludes
- We reach a max and use the result
- We reach a max and indicate an error because we could not converge on a result

All of these can be handled in a `increment` CB and a `end` CB.

>   In CM usage, the latter must occur first, and the second
> signals a failure.

So when the `end` CB is called we either succeeded or had a failure.  The end CB can evaluate the results and determine either one.  The algorithm could also signal to the `end` CB whether the result is a failure or success.

>
>
> Regards,
> Gilles
>
> P.S. I'll commit the "IntegerSequence" class. You can try and patch it
>      (or design an alternative) to show what you mean on some specific
>      CM use of the old "Incrementor".

OK I think it would be fun to play more with this.  I've been meaning to experiment with JDK 8 asynchronous callbacks as well. Hope to get some free time soon!

Cheers,
- Ole


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [math] Negative max in class Incrementor

Posted by Gilles <gi...@harfang.homelinux.org>.
Hi.

On Sat, 29 Aug 2015 22:21:55 -0500, Ole Ersoy wrote:
> I'm deleting most of the discussion, because I think I may be
> throwing too many ingredients on the table.  For further context
> please see the previous thread.
>
>> I don't get that.  Here the main purpose is to set a hard limit
>> that will raise an exception (to avoid that some algo from running
>> forever).
> Well there are two concerns here.  One is the precise number of steps
> that should be executed and the other is whether we need to raise the
> exception.
>
> To stop the algorithm from running forever we let the `end` callback
> notify the thing doing the incrementing that we are done.  Does that
> make sense?

Not sure. The current usage is
1. Set the hard limit (and action when that limit is reached)
2. Let the incrementor do its thing (count and call "trigger").

IIUC, you propose that the increment tells its caller that the
limit has been reached. Then, I suppose, the caller will have to
act, rather than the incrementor.
Unless I'm missing something, I don't see the advantage (in the
examples from CM).

> Secondly suppose we expect a sequence like 5, 10, 15, 20...but the
> max is 17.  Do we loop past 17 and let that be the final loop, thus
> passing in 20 to the increment listener / cb, or do we stop at 15?

The proposed IntegerSequence.Incrementor would trigger the callback
if "increment()" is called again after 15 is returned.
Note that the increment does not stop by itself. If a range is created
its last value will be 15 (and the callback will never be called, by
construction).

> By
> letting the developer calculate the number of steps, avoiding the use
> of a max, we gain simplicity and flexibility.

In all CM usage, the number of steps is unknown; the Incrementor is
intended to avoid an infinite loop.

> Lastly perhaps the `increment` callback wants to notify the
> `incrementer / algorithm` that it's done.  In this case we only
> specify the `start` and `stepSize` arguments and leave it up to the
> `increment` callback to realize the termination point and stop the
> algorithm.

Cf. previous paragraph.
Termination of the counter is not the same as termination of the
algorithm.  In CM usage, the latter must occur first, and the second
signals a failure.

Regards,
Gilles

P.S. I'll commit the "IntegerSequence" class. You can try and patch it
      (or design an alternative) to show what you mean on some specific
      CM use of the old "Incrementor".


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [math] Negative max in class Incrementor

Posted by Ole Ersoy <ol...@gmail.com>.
I'm deleting most of the discussion, because I think I may be throwing too many ingredients on the table.  For further context please see the previous thread.

> I don't get that.  Here the main purpose is to set a hard limit
> that will raise an exception (to avoid that some algo from running
> forever).
Well there are two concerns here.  One is the precise number of steps that should be executed and the other is whether we need to raise the exception.

To stop the algorithm from running forever we let the `end` callback notify the thing doing the incrementing that we are done.  Does that make sense?

Secondly suppose we expect a sequence like 5, 10, 15, 20...but the max is 17.  Do we loop past 17 and let that be the final loop, thus passing in 20 to the increment listener / cb, or do we stop at 15? By letting the developer calculate the number of steps, avoiding the use of a max, we gain simplicity and flexibility.

Lastly perhaps the `increment` callback wants to notify the `incrementer / algorithm` that it's done.  In this case we only specify the `start` and `stepSize` arguments and leave it up to the `increment` callback to realize the termination point and stop the algorithm.

Cheers,
- Ole


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [math] Negative max in class Incrementor

Posted by Gilles <gi...@harfang.homelinux.org>.
On Sat, 29 Aug 2015 15:56:38 -0500, Ole Ersoy wrote:
> On 08/29/2015 06:59 AM, Gilles wrote:
>> On Fri, 28 Aug 2015 21:25:12 -0500, Ole Ersoy wrote:
>>> On 08/28/2015 04:29 PM, Gilles wrote:
>>>> On Fri, 28 Aug 2015 11:48:28 -0500, Ole Ersoy wrote:
>>>>> 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);
>>>>> }
>>>>
>>>> The callback is intended to let the user choose which action the 
>>>> counter
>>>> exhaustion should trigger. IIRC your proposal, this would not be 
>>>> possible
>>>> anymore.
>>> OK - Got it.  I too easily associated the 'MaxCountExceeded' 
>>> callback
>>> with the MaxCountExceededException.  My first thought was that 
>>> (Even
>>> though the doc says otherwise) if the max is exceeded, then this is
>>> bad, and so an exception should be triggered.  How about removing 
>>> the
>>> MaxCountExceededException
>>
>> ?
>> The exception could be used independently of the callback interface.
> If the IntegerSequence becomes an IntegerSequenceEmitter, and in
> general commons math follows a pattern of listening for events like
> `start`, `increment`, and `end` then we will never exceed the max.
>> What would be the gain?
> - Shorter implementation

Of what?

> - No exceptions

Hmm, in this case, the main usage in CM is to throw an exception
when an iterative algorithm exhausts the counter...

> - Event emitter pattern (Which is a popular design pattern for light
> coupling of objects).
> - The event emitter can be extended to add more events
> - We can skip checking `canIncrement()`

But the user code must listen to the events, instead.  No?

> - It introduces a different way of thinking about algorithm design

I'm ready to believe it but are we ready for yet another change
in paradigm within CM?
Already several issues awaits the fluent API upgrade...

You can always formulate a general request, and suggest a plan
for updating the whole library. :-)

>>
>> [Seems overkill for the counter functionality as currently used 
>> within CM.]
> I think we will gain utility, simplify both current and future code,
> and possibly establish a better development pattern for algorithms if
> we use an exception free IntegerSequenceEmitter.
>
>>
>>> P.S.
>>> Plus one for "Incrementor(long start, long max).
>>
>> What do you think of the new API
>>   https://issues.apache.org/jira/browse/MATH-1259
>> ?
>
> I think it should work like this:
> =======================
> Incrementor.build().setStart(start).
> setNumberOfSteps(numberOfSteps)
> .stepSize(stepSize).
> setCallback(CallbackEnum.START, startCB).
> seCallback(CallbackEnum.END, endCB).
> setCallback(CallbackEnum.INCREMENT, incrementCB).
> down()
> ||
> up();
> =======================
>
> If down is called at the end then we get a sequence like 15, 10,
> 5...If up() is called then ...

Method 'up' and 'down' are the equivalent of a positive or negative
increment, respectively.
At first sight, the latter seems more usual in the context of a math
library.
Then, the "stepSize" should be restricted to positive.

>
> Callbacks are optional.  We can call increment or increment(10)
> indefinitely without an exception being thrown.

Not indefinitely, only up to MAX_VALUE...

>  We should be able to
> retrieve the number of increments left, as is included in the current
> design.

We could add that, if the need arises.

> It's up to the developer to calculate the number of steps.  So for
> example if the max is 17 and the start is 5 and the step size is 5,
> then the developer has to decide if she wants 3 or 4 steps completed.

I don't get that.  Here the main purpose is to set a hard limit
that will raise an exception (to avoid that some algo from running
forever).

Regards,
Gilles

>
> Cheers,
> - Ole
>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [math] Negative max in class Incrementor

Posted by Ole Ersoy <ol...@gmail.com>.

On 08/29/2015 06:59 AM, Gilles wrote:
> On Fri, 28 Aug 2015 21:25:12 -0500, Ole Ersoy wrote:
>> On 08/28/2015 04:29 PM, Gilles wrote:
>>> On Fri, 28 Aug 2015 11:48:28 -0500, Ole Ersoy wrote:
>>>> 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);
>>>> }
>>>
>>> The callback is intended to let the user choose which action the counter
>>> exhaustion should trigger. IIRC your proposal, this would not be possible
>>> anymore.
>> OK - Got it.  I too easily associated the 'MaxCountExceeded' callback
>> with the MaxCountExceededException.  My first thought was that (Even
>> though the doc says otherwise) if the max is exceeded, then this is
>> bad, and so an exception should be triggered.  How about removing the
>> MaxCountExceededException
>
> ?
> The exception could be used independently of the callback interface.
If the IntegerSequence becomes an IntegerSequenceEmitter, and in general commons math follows a pattern of listening for events like `start`, `increment`, and `end` then we will never exceed the max.
> What would be the gain?
- Shorter implementation
- No exceptions
- Event emitter pattern (Which is a popular design pattern for light coupling of objects).
- The event emitter can be extended to add more events
- We can skip checking `canIncrement()`
- It introduces a different way of thinking about algorithm design



>
> [Seems overkill for the counter functionality as currently used within CM.]
I think we will gain utility, simplify both current and future code, and possibly establish a better development pattern for algorithms if we use an exception free IntegerSequenceEmitter.

>
>> P.S.
>> Plus one for "Incrementor(long start, long max).
>
> What do you think of the new API
>   https://issues.apache.org/jira/browse/MATH-1259
> ?

I think it should work like this:
=======================
Incrementor.build().setStart(start).
setNumberOfSteps(numberOfSteps)
.stepSize(stepSize).
setCallback(CallbackEnum.START, startCB).
seCallback(CallbackEnum.END, endCB).
setCallback(CallbackEnum.INCREMENT, incrementCB).
down()
||
up();
=======================

If down is called at the end then we get a sequence like 15, 10, 5...If up() is called then ...

Callbacks are optional.  We can call increment or increment(10) indefinitely without an exception being thrown.  We should be able to retrieve the number of increments left, as is included in the current design.

It's up to the developer to calculate the number of steps.  So for example if the max is 17 and the start is 5 and the step size is 5, then the developer has to decide if she wants 3 or 4 steps completed.

Cheers,
- Ole


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [math] Negative max in class Incrementor

Posted by Gilles <gi...@harfang.homelinux.org>.
On Fri, 28 Aug 2015 21:25:12 -0500, Ole Ersoy wrote:
> On 08/28/2015 04:29 PM, Gilles wrote:
>> On Fri, 28 Aug 2015 11:48:28 -0500, Ole Ersoy wrote:
>>> 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);
>>> }
>>
>> The callback is intended to let the user choose which action the 
>> counter
>> exhaustion should trigger. IIRC your proposal, this would not be 
>> possible
>> anymore.
> OK - Got it.  I too easily associated the 'MaxCountExceeded' callback
> with the MaxCountExceededException.  My first thought was that (Even
> though the doc says otherwise) if the max is exceeded, then this is
> bad, and so an exception should be triggered.  How about removing the
> MaxCountExceededException

?
The exception could be used independently of the callback interface.

> and using a more "Stream" like interface?:
>
> emitter.on('start', cb);
> emitter.on('increment', cb);
> emitter.on('end', cb);

What would be the gain?
[Seems overkill for the counter functionality as currently used within 
CM.]

> P.S.
> Plus one for "Incrementor(long start, long max).

What do you think of the new API
   https://issues.apache.org/jira/browse/MATH-1259
?

Gilles


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [math] Negative max in class Incrementor

Posted by Ole Ersoy <ol...@gmail.com>.

On 08/28/2015 04:29 PM, Gilles wrote:
> On Fri, 28 Aug 2015 11:48:28 -0500, Ole Ersoy wrote:
>> 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);
>> }
>
> The callback is intended to let the user choose which action the counter
> exhaustion should trigger. IIRC your proposal, this would not be possible
> anymore.
OK - Got it.  I too easily associated the 'MaxCountExceeded' callback with the MaxCountExceededException.  My first thought was that (Even though the doc says otherwise) if the max is exceeded, then this is bad, and so an exception should be triggered.  How about removing the MaxCountExceededException and using a more "Stream" like interface?:

emitter.on('start', cb);
emitter.on('increment', cb);
emitter.on('end', cb);

Cheers,
- Ole

P.S.
Plus one for "Incrementor(long start, long max).


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [math] Negative max in class Incrementor

Posted by Gilles <gi...@harfang.homelinux.org>.
On Fri, 28 Aug 2015 11:48:28 -0500, Ole Ersoy wrote:
> 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);
> }

The callback is intended to let the user choose which action the 
counter
exhaustion should trigger. IIRC your proposal, this would not be 
possible
anymore.

Regards,
Gilles

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


Re: [math] Negative max in class Incrementor

Posted by Ole Ersoy <ol...@gmail.com>.
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-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


Re: [math] Negative max in class Incrementor

Posted by Gilles <gi...@harfang.homelinux.org>.
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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [math] Negative max in class Incrementor

Posted by Gilles <gi...@harfang.homelinux.org>.
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)?


Gilles


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [math] Negative max in class Incrementor

Posted by Gilles <gi...@harfang.homelinux.org>.
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.

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


Best,
Gilles

> Best regards,
> Gunel Jahangirova.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org