commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ole Ersoy <ole.er...@gmail.com>
Subject Re: [math] Negative max in class Incrementor
Date Fri, 28 Aug 2015 16:48:28 GMT
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


Mime
View raw message