Proposed IntegerSpinner buggy behavior correction - JDK-8242553
kevin.rushforth at oracle.com
Wed Apr 15 22:42:03 UTC 2020
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.
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
> - 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
>> 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.
>>> 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>:
>>>> 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 :
>>>> 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?
More information about the openjfx-dev