Proposed IntegerSpinner buggy behavior correction - JDK-8242553

[Nir Lisker]

I would say that for doubles, the minimum step size is the one given by ulp
= Math.ulp(double). Then the double case should behave like the
integer case where the ulp is 1 I think.
So, for the angle case of [0, 360), if we are at 360 - ulp, incrementing by
N * ulp will get us to (N-1) * ulp.

On Wed, Apr 22, 2020 at 3:41 PM Kevin Rushforth <kevin.rushforth at oracle.com>
wrote:

> Hi Ajit,
>
> Thanks for writing this up. I think we are clear on what should happen
> for IntegerSpinnerValueFactory and ListSpinnerValueFactory. To answer
> your two questions, I'll take the second one first:
>
> > 2. Do we really care about negative values in no of steps OR
> > amountToStepBy?
> > I am asking this as we already have separate increment() and
> > decrement() methods. Should we bar such negative values? That’s
> > another behavior change though :)
>
> Once you switch to a proper modulo function, the easy answer is that
> negative values will "just work". A negative amountToStepBy might even
> make sense in some use cases (think of an increasing debt value), so
> there is no reason not to support it. A negative number of steps doesn't
> really make sense, since if you want to call increment(-N) you could
> equivalently call decrement(N), but again, it will "just work" as
> expected once the wrapping function is fixed. If we were designing from
> scratch I'd specify that nsteps must be >= 0, but it isn't worth it to
> make the change now (which would require a CSR) to prohibit it.
>
> > 1. Need to decide what would be better way to fix
> > DoubleSpinnerValueFactory wrapAround behavior?
>
> It might be helpful to compare it to the Integer flavor. By definition,
> IntegerSpinnerValueFactory represents a set of discrete values: min and
> max are two distinct values, and there is a well-defined ordering when
> wrapping. If you wrap, it should be a continuous (in both positive and
> negative directions) list of values like so:
>
> min, min+1, min+2, ... max-2, max-1, max, min, min+1, min+2, ...
>
> The formula is simple. After incrementing or decrementing by N *
> amountToStepBy, do the following if wrapAround is true:
>
>      if (val < min || val > max) val = (val - min) % (max - min + 1) + min;
>
> The above works regardless of step size because integers are naturally
> discrete values. It is well-behaved and natural to consider max and min
> being 1 apart from each other when wrapping.
>
> By contrast, it's harder to know what the right answer is for
> DoubleSpinnerValueFactory. It's quite possible that in some cases, the
> app expects the values of DoubleValue factory to also be discrete when
> it comes to wrapping, meaning that min and max are distinct values such
> that if your step was 1.0, you might expect to go from max-1.0 to max to
> min to min+1.0 (although even that definition has problems for
> non-integer values of amountToStepBy, but it's (mostly) solvable). It's
> also quite likely for other cases -- especially ones that would lend
> themselves to wrapAround mode in graphical apps -- where it isn't a set
> of discrete values, but rather is such that min and max represent the
> same value (i.e., the same end result). The simplest example is an angle
> in degrees where 0.0 and 360.0 mean the same thing when used as a
> rotation angle. In this latter use case, stepping from 359.0 to 360.0 to
> 0.0 to 1.0 would cause a discontinuous pause in motion if you mapped the
> output to a rotation angle.
>
> I'm not sure what the right answer is, but the latter seems like a valid
> use case and I would guess at least as common as a "set of discrete
> values" for doubles. Another thing to consider is that even in what I
> will call "discrete" mode (no I'm not really proposing that we add a
> property to control it), you have to take the step size into account;
> otherwise when the step size is < 0.5 you will get stuck at max when
> incrementing by 1 amountToStepBy, unless you have special case logic for
> that.
>
> Any thoughts from other developers?
>
> -- Kevin
>