It depends. Some cleanup like this can be good if it reduces overall
maintenance going forward. In many cases, though, it would take more
effort to get it reviewed and prove that there aren't any regressions
than it is worth. In particular, I don't think there is high value in
the sort of automatic refactoring that IDEs like to suggest.
In the specific case of removing a possibly-redundant call to setFocus,
I'm not sure. One thing to consider is the case where the scene on the
stage goes from null to non-null. You would need to make sure that the
behavior doesn't change. Also, there is the case of multiple stages to
consider.
-- Kevin
On 12/9/2019 12:07 PM, Thiago Milczarek Sayao wrote:
While going across the source code I have noticed some code that might be
incorrect , such as:
- WindowStage.setScene() has a call to requestFocus()
I might be missing something, but I think it does not belong there because the
scene is usually set before show(), and while it may be set after the show and
even multiple times, why should it requestFocus() ?
Removing it may also fix https://bugs.openjdk.java.net/browse/JDK-8212060. It
is already fixed on glass side, but still...
- WindowStage.hadleFocusDisabled() calls requestToFront() and requestFocus()
while requestToFront() already calls requestFocus()
- Intellij points me some cleanups such as null checks before instanceof, ifs
that can be merged, etc.
Would those kind of PR be welcome, accepted (if correct) ?
Cheers.