On Tue, 31 Jan 2023 16:42:32 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Michael Strauß has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> refactoring
>
> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 8325:
>
>> 8323: } else if (count == 0) {
>> 8324: set(false);
>> 8325: }
>
> Is it worth adding a check for `count < 0` and logging a warning (possibly
> treating it as if it were 0)? In theory, it shouldn't happen, but if it ever
> did, `focusWithin` would be wrong after that. This could be done as a P4
> follow-up for 21, unless you are certain that it cannot ever happen.
On the one hand, this might make the code a little bit more robust, but on the
other hand, it might hide a bug elsewhere. Surely `count` should never be
negative. I lean slightly towards not protecting code against bugs in this way,
mostly because it might expose potential bugs sooner.
If we want to validate that this method is not called with an incorrect
argument, maybe just throwing an exception would be better.
-------------
PR: https://git.openjdk.org/jfx/pull/993
