[8u] API Request: RT-25613, ObservableValue should have a hasListener(listener) method

[Randahl Fink Isaksen]

I respect your point of view. Thank you for a thorough response.

Randahl 


On 22 Jan 2014, at 12:18, Martin Sladecek <martin.sladecek at oracle.com> wrote:

> On 01/22/2014 11:38 AM, Randahl Fink Isaksen wrote:
>> Hi Martin
>> 
>> Then I respectfully disagree with this design decision. In my point of view, choosing performance over ease of use is rarely a good idea. Here, the performance choice has put us in a situation where no one knows how many JavaFX apps have duplicate listener bugs, and such bugs can be very hard to debug.
>> 
>> Have anyone done any tests that prove that avoiding listener duplicates would lead to a severe performance impact?
> I'm not the original author of the Observable API, but I remember there were many performance tests back at the days before 2.0 release, esp. when such API decision was made. So likely, the tests were done, but the results were not archived or anything.
> 
>> 
>> I understand, that if all observables had thousands of listeners and we searched for a possible duplicate using an inefficient linear search, we would have a problem. But if reality is that very few observables have more than 20 listeners and these could be stored in a map for efficient duplicate checking, then what is the problem?
> Well it's always a balance between performance, dynamic footprint (a set/map for duplicates) and API. Having an API that extensively checks for all the mistakes a developer can do is an extreme case in the same manner as an API that does not check input at all. If a cleanup of some listeners is missing, it's a developer's bug (memory leak) no matter if there a duplicate listener check or not on the subsequent addListener call when the object becomes "valid" again (or whatnot).
> 
> I understand your position, there are APIs I would also like to behave differently, but the decision was already made, it's too late for the discussion. There might be 3rd implementations that use a list a don't guarantee a duplicate check, we don't want to make them suddenly "broken".
> 
> I also disagree that such bugs are hard to debug, a simple printout should do.
> In many cases it would likely just make a computations run more times than necessary. I can imagine doing a duplicate check on something like -Djavafx.debug=true.
> 
> Regards,
> -Martin
> 
> 
>> 
>> Yours
>> 
>> Randahl
>> 
>> 
>>  On 22 Jan 2014, at 11:26, Martin Sladecek <martin.sladecek at oracle.com> wrote:
>> 
>>> The reason why this was decided this way is simple : performance. You usually don't (try to) add a listener twice, so in most cases it doesn't make sense to check for duplicates every time a listener is added. So we currently leave the burden of avoiding duplicates on the developer.
>>> 
>>> -Martin
>>> 
>>> On 01/22/2014 11:23 AM, Randahl Fink Isaksen wrote:
>>>> Hi Martin
>>>> 
>>>> While I agree your proposed solution would work, I still don’t understand why JavaFX should keep on supporting duplicates in listener collections. Can anyone come up with just 1 example of an application that might be depending on having two listeners on the same Observable? E.g. this kind of code:
>>>> 
>>>> myObservable.addListener(myChangeListener); //add it
>>>> myObservable.addListener(myChangeListener); //add it again
>>>> 
>>>> In what kind of situation would this sort of code make any sense?
>>>> 
>>>> If we all feel confident that the presence of duplicates listeners is always an error, I warmly recommend changing the API to be duplicate free.
>>>> 
>>>> Yours
>>>> 
>>>> Randahl
>>>> 
>>>> 
>>>> 
>>>> 
>>>> On 22 Jan 2014, at 11:07, Martin Sladecek <martin.sladecek at oracle.com> wrote:
>>>> 
>>>>> Hi all,
>>>>> I would like to start discussion about an addition to API in Observable, ObservableValue and all Observable collections.
>>>>> There were multiple requests for a way how to avoid duplicates in listeners lists. The way RT-25613 solves this is that it introduces public boolean hasListener(ListenerType listener) which would return true if the provided listener is already registered.
>>>>> 
>>>>> This has one significant drawback that all of Observable* are actually interfaces. Means we can only add hasListener as a defender method. The problem is with the default implementation. We cannot return anything meaningful, so we have to throw an UnsupportedOperationException. The problem is that this might blow up unexpectedly when some "older" Observable implementation is used. Also, it might be easy to miss when implementing the interface, since the IDE might not force you to implement it.
>>>>> 
>>>>> So as an alternative solution, I propose adding something like:
>>>>> 
>>>>> ensureListener(ListenerType listener)
>>>>> 
>>>>> which would make sure the listener is on the list and if a listener is already present, the number of times listener is registered on the Observable will NOT grow after this call.
>>>>> 
>>>>> The default implementation (for Observable) would look like this:
>>>>> 
>>>>> public default void ensureListener(InvalidationListener listener) {
>>>>>    removeListener(listener);
>>>>>    addListener(listener);
>>>>> }
>>>>> 
>>>>> subclasses might do something more effective. The same would apply to ObservableValue and ChangeListener and Observable[List|Set|Map] and [List|Set|Map]ChangeListener.
>>>>> 
>>>>> What do you think?
>>>>> 
>>>>> JIRA link: https://javafx-jira.kenai.com/browse/RT-25613
>>>>> 
>>>>> -Martin
>