On Thu, 1 Dec 2022 10:35:58 GMT, Nir Lisker <[email protected]> wrote:
>> Yeah, I actually copied this pattern from an older method that has since
>> been removed (`isWindowShowing`) in an effort to avoid initializing the
>> property if all you're doing is calling its getter. I personally don't mind
>> either way, but as it seems `Node` goes through every effort to delay
>> initialization, I followed that pattern. It does duplicate the logic of the
>> property which uses `flatMap`s to achieve the same.
>
> I would suggest adding a comment saying that this is done to avoid
> initialization.
>
> I'm not sure that it's that critical for performance, to be honest.
I think the pattern is good to do, the `shownProperty` will only be used for a
small number of nodes, and not initializing it when not needed is a common
pattern in `Node` itself already. Here for example for `opacity`:
private DoubleProperty opacity;
public final void setOpacity(double value) {
opacityProperty().set(value);
}
public final double getOpacity() {
return opacity == null ? 1 : opacity.get();
}
But it is far from the only one. `disabled` does it, `pickOnBounds`,
`blendMode`, etc.
None of these contain a comment as to why, but I'll add one for `shown`.
-------------
PR: https://git.openjdk.org/jfx/pull/830