On Mon, 14 Nov 2022 20:26:54 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ListenerHelper.java
>> line 70:
>>
>>> 68: * </ul>
>>> 69: */
>>> 70: public class ListenerHelper implements IDisconnectable {
>>
>> This class is mixing two things-
>> 1. Managing listeners in a generic way - by providing ability to add &
>> removeAll (via disconnect())
>> 2. Trying to cater to *Skin specific requirement
>>
>> Ideally, (2) above should be done separately. I mean, the class
>> ListenerHelper should not use `SkinBase`.
>> What do you think?
>
> you are correct: the original intent for this class was to make it a general
> purpose facility to help with listeners, something that might be useful at
> the application level (and it used `ListenerHelper.get(Node)`). Since that
> would require CSR and a public discussion, we decided to hide it as an
> implementation detail in skins (to unblock skin memory leak fixes).
>
> Once we go through all the necessary discussions and everyone agrees, we
> could introduce it as a public API in the javafx.base module.
Exactly. In its current form, `ListenerHelper` is a utility helper class for
Skins, and should be reviewed with that in mind. As discussed in previous
comments, there are several things that will need to change when/if this is
proposed as a more general utility. We can defer that discussion, since this is
entirely an internal helper class for now.
As a next step, after this is integrated and before any discussion of making
this a public utility, it can be used as a replacement for
`LambdaMultiplePropertyChangeListenerHandler` -- see
[JDK-8296076](https://bugs.openjdk.org/browse/JDK-8296076).
-------------
PR: https://git.openjdk.org/jfx/pull/908
