I respect your point of view. Thank you for a thorough response.
Randahl
On 22 Jan 2014, at 12:18, Martin Sladecek <martin.slade...@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.slade...@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.slade...@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
>
