On Thu, 1 Dec 2022 10:35:58 GMT, Nir Lisker <nlis...@openjdk.org> 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

Reply via email to