On Mon, 29 Apr 2024 17:53:50 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
>> I applied the software engineering principle to leave the code cleaner than >> seen. (Martin Fowler et all) >> >> Should I revert this? > > Sorry for not being clear. I just wanted to point out that it is not required > for the issue fix. > Please don't revert. This looks better. This is the sort of tangentially-related change that should usually be avoided. In this specific case, `clamp(min, val, max)` will produce the same results as `max(start, min(val, end))`, but it isn't guaranteed to do so for malformed input where `max < min`. Given that `text.length()` always returns a non-negative number, the first call to `clamp` is identical to the existing code. And since the resulting` start` value is guaranteed to be `<= length`, the second call to `clamp` is also equivalent. But... reviewers shouldn't have to spend time verifying this to ensure no regression. Since Ambarish is the primary reviewer of this, and he is OK with this change (and I already did spend the time to verify that it is equivalent), then I don't insist that you revert it. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1442#discussion_r1586241421