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

Reply via email to