On Tue, 9 Feb 2021 20:43:08 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> hmm ... might appear convenient (in very controlled contexts) but looks like
>> a precondition violation: the sender of the change must not be null
>> (concededly not explicitly spec'ed but logically implied, IMO)
>>
>> so would tend to _not_ see this as blueprint for a general pattern fx code
>> base
>
> So, what you are suggesting is to replace the above single line to this:
>
> Scene scene = node.getScene();
>
> if (scene != null) {
> Window window = scene.getWindow();
>
> scene.addListener(nodeSceneChangedListener);
>
> if (window != null) {
>
> window.showingProperty().addListener(windowShowingChangedListener);
> }
> }
>
> updateTreeShowing();
>
> And something similarly wordy in dispose again -- and donot forget that I
> would then need to also duplicate the testing code to make sure all these
> branches are covered in both the constructor and in dispose again.
>
> Are you sure? I could extract the methods instead if you donot like me
> passing in `null` for the `ObservableValue`, it would then look like:
>
> sceneChanged(null, node.getScene()); // register chain
>
> sceneChanged(node.getScene(), null); // unregister chain
>
> No further changes in testing required as it is all covered and proven to
> work correctly.
>
> Also I should mention that this is not Skin code in case it was missed.
I could also split the register/unregister part into separate methods for Scene
and Window (as you can see, I have a great aversion against duplicating code,
mostly because it would then need to be tested multiple times as well).
-------------
PR: https://git.openjdk.java.net/jfx/pull/185
