On Fri, 2 Dec 2022 01:06:16 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Adjust Node >> >> - Fixed javadoc >> - Added comment for code that avoid eager instantiation >> - Changed `isShown` to use property if it is available > > modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 1443: > >> 1441: .flatMap(Window::showingProperty); // note: the null >> case is handled by the delegate with an orElse(false) >> 1442: >> 1443: shown = new ReadOnlyBooleanDelegate(Node.this, "shown", >> ov); > > `shownProperty()` might be used quite a lot, have you considered using a > specialized implementation that doesn't need several intermediate observable > values? I think that this is already optimized enough by being lazily instantiated (ie. it won't be created if never used). As a second layer, these are fluent bindings which only register listeners when observed. Creating a specialized implementation should IMHO replicate the lazy behavior (to avoid too many listeners on window/scene when the property isn't actually observed) which is non-trivial and would require an extensive test. This risks creating bugs now (or later) of which there have been quite a few with similar custom properties -- it's easy to accidentally cache an "old" value or forgetting to clear a cached "current" value when invalidated. In short, I think optimized custom implementations have been and still are a source of bugs, with no evidence that these optimizations are worth while.. ------------- PR: https://git.openjdk.org/jfx/pull/830
