commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles <gil...@harfang.homelinux.org>
Subject Re: [math] Negative max in class Incrementor
Date Fri, 28 Aug 2015 21:29:02 GMT
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


Mime
View raw message