On Sun, 9 Jul 2023 23:13:20 GMT, Jose Pereda <[email protected]> wrote:
>> There is no need, the returned `Subscription` is implemented using a lambda
>> that captures the listener (as it needs it to call `removeListener`). As
>> long as you have the `Subscription` referenced, it can't get GC'd. If you
>> let the `Subscription` get GC'd, then you'd lose your means to unsubscribe
>> the listener, just like if you lost the reference to the listener itself,
>> you would never be able to remove it anymore.
>>
>> Weak listeners I would not recommend to use in new code if you can avoid it.
>> They're unpredictable as to when they're removed exactly, and can still
>> receive notifications when your application may be in a state where you
>> don't expect them anymore.
>>
>> The `Subscription` interface is in fact intended to make management of
>> listeners easier so you can rely less on weak listeners. One pattern I can
>> really recommend to use is to automatically disable listeners when the UI
>> they belong with becomes invisible, for example:
>>
>> private Subscription subscription = Subscription.EMPTY;
>>
>> node.sceneProperty()
>> .flatMap(Scene::windowProperty)
>> .flatMap(Window::showingProperty)
>> .subscribe(visible -> {
>> if (visible) {
>> subscription = Subscription.combine(
>> // create subscriptions (to external resources) here
>> as we just became visible!
>> );
>> }
>> else {
>> // we became invisible, remove listeners (from external
>> resources) to remove hard references that
>> // may keep the UI referenced; the other branch will
>> restore them if the UI becomes visible again
>> subscription.unsubscribe(); // unsubscribes everything in
>> one go
>> }
>> });
>>
>> Or you can use `ObservableValue#when` to do something similar, not requiring
>> subscriptions.
>>
>> I suspect it may also prove a useful pattern for `Skin`s that have a
>> specific lifecycle. They can register listeners (and combine in a single
>> subscription) in their constructor / initializer, and remove them all in
>> their `dispose` method.
>
> Yes, `flatMap` or `when` are great for those cases, so the pattern with the
> new `subscribe` proposal seems quite useful.
I missed the `orElse(false)` in the example above, which deals with scene being
`null`. Full example should be:
private Subscription subscription = Subscription.EMPTY;
node.sceneProperty()
.flatMap(Scene::windowProperty)
.flatMap(Window::showingProperty)
.orElse(false)
.subscribe(visible -> {
if (visible) {
subscription = Subscription.combine(
// create subscriptions (to external resources) here as we
just became visible!
);
}
else {
// we became invisible, remove listeners (from external
resources) to remove hard references that
// may keep the UI referenced; the other branch will restore them
if the UI becomes visible again
subscription.unsubscribe(); // unsubscribes everything in one go
}
});
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1257565724
