Mario,
thanks for your suggestion, but I think your specific case will not
justify the change. First of all, as it was already said, equals should
be symmetric. Knowing the implementation, you could rely on the fact
that either the equals is called on the provided listener or on the
listeners already in the list. Might be that somebody else would need
the asymmetry to be implemented in the "listener" provided to
removeListener() method and the current situation suits him, so why do
the change for your case?
Anyway, relying on some implementation-specific behaviour is generally a
sign of bad code practice, so it should give you a hint that you should
find a different solution (like the one you discussed with Tomas
already). It might happen that the code would be changed / re-factored
in the future or some bug would be fixed there and behaviour will change
again, breaking your application in some future JFX version.
Regards,
-Martin
On 22.3.2014 15:47, 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.rushfo...@oracle.com<mailto:kevin.rushfo...@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.mik...@gmail.com><mailto:tomas.mik...@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.mik...@gmail.com><mailto:tomas.mik...@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
<ma...@datenwort.at><mailto:ma...@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
