On Fri, 14 Jul 2023 14:59:00 GMT, John Hendrikx <[email protected]> wrote:
>> I also hadn't considered potential usage of Subscription outside of
>> observables, but I can see some uses for it.
>>
>> John: I would support moving it to javafx.util (via a separate issue) if you
>> think it is worth doing.
>
> Correct packaging is often overlooked, so I think it is worth doing. I'll
> test to see if it doesn't cause any problems, and if not, I'll create an
> issue for it.
> Good point, I didn't think about subscriptions outside of the listeners
> contexts, let alone outside of the JavaFX content. It does look now like it
> belongs in the utils package. Maybe a quick correction can be made if Kevin
> agrees with this and you think it's worth it.
>
> Can you think where else in JavaFX subscriptions could be used? I'm thinking
> about the deprecated `finalize` and try-with-resource, anywhere where we need
> to release resources. This would give a better idea of what the class
> revolves around and thus what documentation fits it.
It's very general, basically a specifically purposed `Runnable` where `run` is
called `unsubscribe` with some chaining methods. You could use it for purposes
we may not be able to think of right now. I've looked at some `dispose`
methods, which often have clean-up code for some inspiration:
1. Use it to clean up references: `() -> { this.skinnable = null;
this.scrollBar = null; }`
2. Use it to stop animation timers or timelines : `() -> animationTimer.stop();`
3. You could wrap other add/remove style API that don't offer a `Subscription`:
`() -> getSkinnable().removeEventHandler( ... )`
I'm not saying you always should do this, but if you are releasing a
`Subscription` anyway, you could combine some of these. Also, if new API
decides to provide a subscription, they're not limited to only remove a
listener, they could also stop a thread, a timer or release some resources that
was started/allocated when the subscription was created.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1263857263
