Ability to decorate ChangeListener

[Kevin Rushforth]

What if changeListeners[index] is null? Maybe this is already illegal....

Anyway, let's see what Martin has to say. In the mean time you file a 
JIRA enhancement request (issuetype=Tweak) if you like.

-- Kevin


Mario Ivankovits wrote:
> The only thing which I ask for is to flip this „if" in the 
> *ExpressionHelper classes:
> So, JavaFX does not break anything and it is up to the app developer 
> to take the risk or not.
>
>>>>>>                    if (listener.equals(changeListeners[index])) {
>>>>>>
>>>>>> If we flip this to
>>>>>>
>>>>>>                if (changeListeners[index].equals(listener)) 
>
>
>
> Am 22.03.2014 um 15:42 schrieb Kevin Rushforth 
> <kevin.rushforth at oracle.com <mailto:kevin.rushforth at oracle.com>>:
>
>> If you are talking about a change to the JavaFX API, we are not 
>> likely to accept such a change if it breaks the contract of equals() 
>> or hashcode(). In your own app it may not matter, but it is dangerous 
>> to assume it won't matter to anyone. Martin owns the core libraries 
>> and can comment further.
>>
>> -- Kevin
>>
>>
>> Mario Ivankovits wrote:
>>> Hi Thomas!
>>>
>>> Thanks for your input. Because I want to decorated listeners added by JavaFX core I can not use the sub/unsub pattern.
>>> Your second proposal is almost what I do right now. In removeListener I consult a map where the decorated listeners and their undecorated one lives.
>>>
>>> Regarding the symmetric doubts. Such listeners will always be removed by passing in the undecorated object to removeListener.
>>> They should act like a transparent proxy.
>>>
>>> Even if this breaks the equals paradigm, in this special case I can perfectly live with an equals/hashCode implementation like this below.
>>> It won’t break your app; as long as both objects do not live in the same HashSet/Map …. for sure - but why should they? 
>>>
>>>     @Override
>>>     public boolean equals(Object obj)
>>>     {
>>>         return obj == delegate; // && this == obj
>>>     }
>>>
>>>     @Override
>>>     public int hashCode()
>>>     {
>>>         return delegate.hashCode();
>>>     }
>>>
>>>
>>> Regards,
>>> Mario
>>>
>>>
>>> Am 22.03.2014 um 14:22 schrieb Tomas Mikula <tomas.mikula at gmail.com>:
>>>
>>>   
>>>> A simpler and more universal solution is to also override removeListener:
>>>>
>>>> public void removeListener(ChangeListener<? super T> listener) {
>>>>    super.removeListener(decorated(listener));
>>>> }
>>>>
>>>> And the equals() method on decorated listeners only compares the
>>>> delegates (thus is symmetric).
>>>>
>>>> Regards,
>>>> Tomas
>>>>
>>>>
>>>> On Sat, Mar 22, 2014 at 2:07 PM, Tomas Mikula <tomas.mikula at gmail.com> wrote:
>>>>     
>>>>> The suspicious thing about your solution is that your smart
>>>>> implementation of equals() is not symmetric.
>>>>>
>>>>> In case the observable value is visible only within your project, you
>>>>> could do this:
>>>>>
>>>>>    interface Subscription {
>>>>>        void unsubscribe();
>>>>>    }
>>>>>
>>>>>    class MyObservableValue<T> implements ObservableValue<T> {
>>>>>        public Subscription subscribe(ChangeListener<? extends T> listener) {
>>>>>            ChangeListener<T> decorated = decorate(listener);
>>>>>           addListener(decorated);
>>>>>           return () -> removeListener(decorated);
>>>>>        }
>>>>>    }
>>>>>
>>>>> and use subscribe()/unsubscribe() instead of addListener()/removeListener():
>>>>>
>>>>>    Subscription sub = observableValue.subscribe(listener);
>>>>>    // ...
>>>>>    sub.unsubscribe();
>>>>>
>>>>> Of course this is not possible if you need to pass the observable
>>>>> value to the outside world as ObservableValue.
>>>>>
>>>>> Regards,
>>>>> Tomas
>>>>>
>>>>> On Sat, Mar 22, 2014 at 6:57 AM, Mario Ivankovits <mario at datenwort.at> wrote:
>>>>>       
>>>>>> Hi!
>>>>>>
>>>>>> In one of my ObservableValue implementations I do have the need to decorate ChangeListener added to it.
>>>>>> Today this is somewhat complicated to implement, as I have to keep a map of the original listener to the decorated one to being able to handle the removal process of a listener. Because the outside World did not know that I decorated the listener and the instance passed in to removeListener ist the undecorated one.
>>>>>>
>>>>>> We can make things easier with a small change in ExpressionHelper.removeListener. When iterating through the listener list, the listener passed in as argument to removeListener is asked if it is equal to the one stored
>>>>>>
>>>>>>                    if (listener.equals(changeListeners[index])) {
>>>>>>
>>>>>> If we flip this to
>>>>>>
>>>>>>                if (changeListeners[index].equals(listener)) {
>>>>>>
>>>>>> a listener decoration can be smart enough to not only check against itself, but also against its delegate.
>>>>>>
>>>>>> What do you think?
>>>>>>
>>>>>> I could prepare a patch for the *ExpressionHelper classes.
>>>>>>
>>>>>>
>>>>>> Best regards,
>>>>>> Mario
>>>>>>
>>>>>>
>>>>>>         
>>>
>>>   
>