Proposed IntegerSpinner buggy behavior correction - JDK-8242553

[Jeanette Winzenburg]

part of the confusion might be the reporter's workaround in  
https://bugs.openjdk.java.net/browse/JDK-8193286 which effectively  
clamps (doesn't make a difference, having amountPerStep=1 and not  
incrementing programatically)

commented that bug with an alternative implemenation and a couple of  
tests (for the most simple case ;)

-- Jeanette

Zitat von Kevin Rushforth <kevin.rushforth at oracle.com>:

> I just took another look at the SpinnerValueFactory API docs. The  
> use of the term "circular" heavily implies modulo arithmetic as the  
> expected behavior if wrapAround is true. That the usual meaning of  
> "wrap" versus "clamp" when you have a range bounded on both ends.  
> Maybe the confusion comes from the fact that it isn't stated as  
> clearly as it might be, coupled with a single example of a step by  
> one going from max to min (if incrementing). I note that example  
> doesn't say what the amountToStepBy is, but it wasn't trying to be  
> prescriptive.
>
> Since the current behavior of "wrap around unless you happen to wrap  
> a bit too much and then we'll clamp" doesn't make sense in any  
> universe that I can think of, it is fine to treat this as a bug. I'm  
> not worried about "bug compatibility" here.
>
> Clarifying the spec at the same time seems like a good idea to me. A  
> related issue is that the default value of wrapAround is not  
> specified (the default is `false`, but you wouldn't know that from  
> reading the docs). This should also be addressed at the same time.
>
> You mentioned that this is specific to IntegerValueFactory, but it  
> looks like DoubleValueFactory (and maybe ListValueFactory) have the  
> same bug. Or am I missing something?
>
> On an unrelated note, I just noticed that the SpinnerValueFactory  
> constructor has no documentation. This is because it is an  
> implicitly added constructor, which is an anti-pattern for public  
> API. I'll file a separate issue for that.
>
> -- Kevin
>
>
> On 4/15/2020 2:23 AM, Jeanette Winzenburg wrote:
>> Hi Ajit,
>>
>> yes, I read the doc, probably a bit differently - could well be my  
>> misunderstanding and misunderstandable wording :)
>>
>> Trying again:
>>
>> - I read your suggestion (in  
>> https://bugs.openjdk.java.net/browse/JDK-8242553) to imply f.i.  
>> that being at value and incrementing a full-cycle (that is max -min  
>> +1), I will land on value again
>> - for me the doc seemed to imply that in such a case I would land  
>> on min. Though, given the "circular" as you pointed out correctly,  
>> was my misunderstanding
>> - the current implementation is buggy  
>> (https://bugs.openjdk.java.net/browse/JDK-8193286) in calculating  
>> the remainder (which is what the first bullet amounts to)  
>> incorrectly for min != 0
>>
>> Where do I err?
>>
>> -- Jeanette
>>
>>
>> Zitat von Ajit Ghaisas <ajit.ghaisas at oracle.com>:
>>
>>> Hi Jeanette,
>>>
>>> The doc never assumes amountPerStep = 1. I am quoting it here -
>>> “The wrapAround property is used to specify whether the value  
>>> factory should be circular. For example, should an integer-based  
>>> value model increment from the maximum value back to the minimum  
>>> value (and vice versa).”
>>>
>>> The word “circular” clarifies that once we exceed maximum value,  
>>> we should start from minimum.
>>> I think, the doc is OK in it’s current form, but implementation  
>>> needs to be corrected.
>>>
>>> Regards,
>>> Ajit
>>>
>>>
>>>> On 14-Apr-2020, at 8:01 PM, Jeanette Winzenburg  
>>>> <fastegal at swingempire.de> wrote:
>>>>
>>>>
>>>> Hi Ajit,
>>>>
>>>> thought the doc was simply bad (in specifying the behavior for  
>>>> amountPerStep = 1 and not thinking of larger amounts) - my  
>>>> expection is a calculated wrap, that is the target as you suggest  
>>>> via modulo the difference from current value. Don't know if  
>>>> anybody took the doc literally ..
>>>>
>>>> -- Jeanette
>>>>
>>>> Zitat von Ajit Ghaisas <ajit.ghaisas at oracle.com>:
>>>>
>>>>> Hi,
>>>>>
>>>>>   Once I fix JDK-8193286, I would like to take up JDK-8242553  
>>>>> (IntegerSpinner does not wrap around values correctly if  
>>>>> amountToStepBy is larger than total numbers between Max and Min)
>>>>>
>>>>>   The current implementation is not as per what is documented.
>>>>>   Refer :  
>>>>> https://openjfx.io/javadoc/14/javafx.controls/javafx/scene/control/SpinnerValueFactory.html#wrapAroundProperty
>>>>>
>>>>>   I propose to fix the current buggy behavior of IntegerSpinner.
>>>>>   Although it is a corner case, I would like to know if anybody  
>>>>> relies on this buggy behavior?
>>>>>
>>>>> Regards,
>>>>> Ajit
>>>>
>>>>
>>>>
>>
>>
>>