On Thu, 13 Oct 2022 23:45:10 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
> 1. it looks like your proposal will work, as long as the `active` property is
> not referenced outside of its owner (in our discussion, Skin). If that is
> true, as soon as you set active=false, the listener is disconnected and the
> skin, the active property, and the lambda can be collected. Thus, as you
> correctly explained, we need to create a large aggregate with two flat maps
> in order to avoid the memory leak (whether you hide this complexity behind
> shownProperty is irrelevant).
Yes, the `shownProperty` is there purely for convenience, and more useful for
regular controls, not the skin scenario I think. I have this helper that I use
for this purpose at the moment:
public static ObservableValue<Boolean> showing(Node node) {
return node.sceneProperty()
.flatMap(Scene::windowProperty)
.flatMap(Window::showingProperty)
.orElse(false);
}
> So yes, it will work, as long as the developer makes no mistakes and does
> not wire the thing directly (a mistake I readily made)
Yes, that's certainly true, I think the `shownProperty` will help with that
though, as I think it will prevent people from storing the flatMap aggregate
and reusing it, and perhaps then reusing it for the wrong controls. Reuse
however can still be fine for groups of listeners of several controls that go
together. If your control's lifecycle is tied to a group, container or dialog,
then it is perfectly fine to use one of their shown properties.
> 2. as a side note, I would discourage a pattern where Nodes are reused and
> need to be reconnected. at least in my applications, I never do that, but I
> can see situation when this might be useful.
I'm not in favor of that either, and I don't think I ever do it, but it is
allowed by JavaFX, and so if you did, then the listeners would be restored as
they were, and since you had to have a reference to this reused control still,
nothing will have been GC'd yet.
> 3. is when() a good name? it sort of implies a time-domain criterion instead
> of when a boolean becomes true (whenTrue? whenAllowed?) i could be wrong
> here.
ReactFX used `conditionOn` and had a special version `conditionOnShowing` which
accepts a `Node` (this would however create a circular reference between
projects base and graphics). I proposed `when` as it is nice and short,
inspired by the recent developments in the area of switch expressions. Even
better would be `while` IMHO, but that is unfortunately a reserved keyword.
Still, when works reasonably well: "Listen to changes of this long lived
property **when** this condition holds". While would definitely be better or
perhaps "as long as" or "whenever" :)
I don't think we should add "true" in the name, no other conditionals do this
(like `Stream#filter` or `List#removeIf`).
`filter` itself is also not an option, as this has a different meaning which we
may implement in the future. `filter` would allow you to remove certain values,
which would set the value to empty:
textProperty.filter(text ->
!isRudeWord()).orElse("<censored>").addListener(...);
-------------
PR: https://git.openjdk.org/jfx/pull/830
