Re: Ability to decorate ChangeListener
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.commailto: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.commailto: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.commailto: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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT 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.atmailto: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
Re: Ability to decorate ChangeListener
Thanks for your answer!
One thing, I think, we can agree on, is that it is overly complex for an OO
language to decorate things like these listeners.
What about introducing another interface like ChangeListenerDecoration with a
special named „boolean decorates(ChangeListener)) method and call this if the
listeners hold by *ExpressionHelp class implements that interface?
BTW: If we care about symmetry, why not change the
listener.equals(otherListener) to listener == otherListener at all? Then, there
are not implementation details one can make use of.
I even might argue that it is a security risk to use the .equals() the way it
is now, as an evil listener implementation is able to remove all listeners from
the list by simply returning true from equals()
It should be the responsibility of the listener in the list to know if the
passed in listener justify the removal of itself, right? … from a security
standpoint! :-)
Regards,
Mario
Am 24.03.2014 um 07:31 schrieb Martin Sladecek martin.slade...@oracle.com:
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.commailto: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.commailto: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.commailto: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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT decorated = decorate(listener);
addListener(decorated);
return () - removeListener(decorated);
}
}
and use subscribe()/unsubscribe() instead of
Re: Ability to decorate ChangeListener
On 24.3.2014 15:24, Mario Ivankovits wrote: But, after this discussion I do not see why one ever used .equals() at all. Look, it does not fit my needs, I do not see any use-case where one would add an removeListener with asymmetric .equals() and thus it is better you use == I think. This clarifies that EXACTLY the added listener instance is required to remove any listener AND it gives no room to discussions like we had because the indention is perfectly clear - even to those reading JavaFX core code and bug fixing things in JavaFX. For me this would be fine and understandable - I will go the decorator-cache-map so I will be fine always. One example might be in bidirectional binding. There's a special listener that takes the 2 properties that are bound together and it's equals returns true if the other listener is of the same class and it's two properties (no matter in what order) are identical. This way, you can just write Bindings.unbindBidirectional() and the internal implementation does not need to remember the listener instance anywhere. The equals() is symmetric in this case, but == would not work here. -Martin
Re: Ability to decorate ChangeListener
Am 24.03.2014 um 15:36 schrieb Martin Sladecek martin.slade...@oracle.com: On 24.3.2014 15:24, Mario Ivankovits wrote: But, after this discussion I do not see why one ever used .equals() at all. Look, it does not fit my needs, I do not see any use-case where one would add an removeListener with asymmetric .equals() and thus it is better you use == I think. This clarifies that EXACTLY the added listener instance is required to remove any listener AND it gives no room to discussions like we had because the indention is perfectly clear - even to those reading JavaFX core code and bug fixing things in JavaFX. For me this would be fine and understandable - I will go the decorator-cache-map so I will be fine always. One example might be in bidirectional binding. There's a special listener that takes the 2 properties that are bound together and it's equals returns true if the other listener is of the same class and it's two properties (no matter in what order) are identical. This way, you can just write Bindings.unbindBidirectional() and the internal implementation does not need to remember the listener instance anywhere. The equals() is symmetric in this case, but == would not work here. -Martin Ah - Thanks for this lesson! :-) OMG … just looked up the code. The implementation of BidirectionalBinding uses weak references and it might happen that you ExpressionHelperBase.trim() them out of the listener list if either side of the bound property got garbage collected. This means, if I decorated them, they will stay in my map because there is add/removeListener asymmetry. (just to use the word symmetry again ;-) ) So, it is even more complicated to decorate that stuff. How much I’d love to see a DecoratedListener interface like you have for WeakListener … but yay … :-( Thanks for listening anyway! :-) Regards, Mario
Re: Ability to decorate ChangeListener
On Mon, Mar 24, 2014 at 3:24 PM, Mario Ivankovits ma...@datenwort.at wrote:
Hi Tomas!
No worries, I’ll go the way on my own there then.
But, after this discussion I do not see why one ever used .equals() at all.
Look, it does not fit my needs, I do not see any use-case where one would add
an removeListener with asymmetric .equals() and thus it is better you use ==
I think.
As Martin pointed out, sometimes == doesn't work for you and you need
(symmetric) equals(). Another example is my proposed solution #2 to
your problem.
Also, using equals instead of identity comparison is consistent with
how Collection.remove() determines equality.
This clarifies that EXACTLY the added listener instance is required to remove
any listener AND it gives no room to discussions like we had because the
indention is perfectly clear - even to those reading JavaFX core code and bug
fixing things in JavaFX.
For me this would be fine and understandable - I will go the
decorator-cache-map so I will be fine always.
BTW: There are always implementation details one has to assume. For example,
I have to assume that the internals will always use the add/removeListener
methods and not modifying the list (or whatever) directly.
There is something which we need to hold on - even if there is no
documentation for things like this.
Any yay, if you patch some JavaFX core stuff, you will get deeper insights
and do not need to assume anymore, you KNOW how it works. Sometimes it is not
easy to misuse things then to get your job done! ;-)
Also, if there is a risk in there current implementation it does not help
killing the discussion with the argument that there are other risks too.
I am pretty well aware about stuff like that.
PS: I would love to explain in detail why I decorate change listener, but
this is out of scope of this thread.
Regards,
Mario
Am 24.03.2014 um 14:04 schrieb Tomas Mikula tomas.mik...@gmail.com:
Hi Mario,
On Mon, Mar 24, 2014 at 8:46 AM, Mario Ivankovits ma...@datenwort.at wrote:
Thanks for your answer!
One thing, I think, we can agree on, is that it is overly complex for an OO
language to decorate things like these listeners.
I'm curious why you need to decorate the listeners in the first place.
What about introducing another interface like ChangeListenerDecoration with
a special named „boolean decorates(ChangeListener)) method and call this if
the listeners hold by *ExpressionHelp class implements that interface?
If you are writing your own ObservableValue implementation, you can as
well implement your own ExpressionHelper that does whatever you want
(just that you don't forget this option ;-)).
BTW: If we care about symmetry, why not change the
listener.equals(otherListener) to listener == otherListener at all? Then,
there are not implementation details one can make use of.
I even might argue that it is a security risk to use the .equals() the way
it is now, as an evil listener implementation is able to remove all
listeners from the list by simply returning true from equals()
It should be the responsibility of the listener in the list to know if the
passed in listener justify the removal of itself, right? … from a security
standpoint! :-)
You cannot secure your application at this level. An evil listener
might as well introduce memory leaks, cause stack overflow, or wipe
out your hard disk.
Tomas
Regards,
Mario
Am 24.03.2014 um 07:31 schrieb Martin Sladecek martin.slade...@oracle.com:
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
Re: Ability to decorate ChangeListener
I'm pretty sure this discussion has solidified around not flipping the
equals() but this is a good example of something that we wouldn't
change. If you write code that relies on this, it will break in the
future when new code is added in FX that does not follow the pattern.
Steve
On 2014-03-22 10:47 AM, 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.commailto: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.commailto: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.commailto: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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT 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.atmailto: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
Re: Ability to decorate ChangeListener
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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT 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 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
Re: Ability to decorate ChangeListener
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 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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT 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 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
Re: Ability to decorate ChangeListener
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:
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 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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT 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 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
Re: Ability to decorate ChangeListener
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:
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 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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT 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 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
Re: Ability to decorate ChangeListener
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.commailto: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.commailto: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.commailto: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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT 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.atmailto: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
Re: Ability to decorate ChangeListener
Yep, null throws a NullPointerException in addListener by design (also in
removeListener) - the listener collection is safe.
Am 22.03.2014 um 15:50 schrieb Kevin Rushforth
kevin.rushfo...@oracle.commailto:kevin.rushfo...@oracle.com:
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.rushfo...@oracle.commailto: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.commailto: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.commailto: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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT 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.atmailto: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
Re: Ability to decorate ChangeListener
Hi Mario,
even if your proposal gets accepted, you would still depend on an
implementation detail of JavaFX, which is always good to avoid.
To sum up, my second proposal compared to your current solution:
(+) your equals() stays symmetric;
(+) no need for an extra Map of listeners.
My second proposal compared to your desired solution if your proposal
is accepted:
(+) your equals() stays symmetric;
(+) you don't depend on an implementation detail in JavaFX.
Best,
Tomas
On Sat, Mar 22, 2014 at 3:47 PM, Mario Ivankovits ma...@datenwort.at 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:
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:
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
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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT 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
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
Re: Ability to decorate ChangeListener
Probably you avoid the map, but it requires to decorate the listener again,
right?
If you think about the tons of observable values and add/removeListener
operations in an tree view when scrolling it might get costly and put pressure
on the GC.
Anyway, I will be more than happy to assist with a cleaner solution by
introduction an interface like DecoratedChangeListener which provides a special
.equalsChangeListener(ChangeListener other) method.
Am 22.03.2014 um 15:55 schrieb Tomas Mikula
tomas.mik...@gmail.commailto:tomas.mik...@gmail.com:
Hi Mario,
even if your proposal gets accepted, you would still depend on an
implementation detail of JavaFX, which is always good to avoid.
To sum up, my second proposal compared to your current solution:
(+) your equals() stays symmetric;
(+) no need for an extra Map of listeners.
My second proposal compared to your desired solution if your proposal
is accepted:
(+) your equals() stays symmetric;
(+) you don't depend on an implementation detail in JavaFX.
Best,
Tomas
On Sat, Mar 22, 2014 at 3:47 PM, Mario Ivankovits
ma...@datenwort.atmailto:ma...@datenwort.at 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.commailto: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.commailto: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.commailto: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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT 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.atmailto: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
Re: Ability to decorate ChangeListener
On Sat, Mar 22, 2014 at 4:11 PM, Mario Ivankovits ma...@datenwort.at wrote:
Probably you avoid the map, but it requires to decorate the listener again,
right?
Right. For a pair of addListener-removeListener calls, you have 2
decorations instead of 1. I usually think of the costs in asymptotic
terms: if 1 decoration is cheap, then 2 decorations are cheap as well.
Or, if we can afford n decorations, then we can probably afford 2n
decorations. I know this is not quite true in real world (200ms is not
the same as 100ms), but is a good way to think about complexity.
If you think about the tons of observable values and add/removeListener
operations in an tree view when scrolling it might get costly and put
pressure on the GC.
removeListener creates one extra short-lived object. I don't know if
it is still true for the current garbage collector in Hotspot, but a
while ago I read that creating a lot of cheap short-lived objects is
fine for the GC.
Regards,
Tomas
Anyway, I will be more than happy to assist with a cleaner solution by
introduction an interface like DecoratedChangeListener which provides a
special .equalsChangeListener(ChangeListener other) method.
Am 22.03.2014 um 15:55 schrieb Tomas Mikula tomas.mik...@gmail.com:
Hi Mario,
even if your proposal gets accepted, you would still depend on an
implementation detail of JavaFX, which is always good to avoid.
To sum up, my second proposal compared to your current solution:
(+) your equals() stays symmetric;
(+) no need for an extra Map of listeners.
My second proposal compared to your desired solution if your proposal
is accepted:
(+) your equals() stays symmetric;
(+) you don't depend on an implementation detail in JavaFX.
Best,
Tomas
On Sat, Mar 22, 2014 at 3:47 PM, Mario Ivankovits ma...@datenwort.at
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:
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:
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
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 MyObservableValueT implements ObservableValueT {
public Subscription subscribe(ChangeListener? extends T listener) {
ChangeListenerT 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
wrote:
Hi!
In one of my ObservableValue implementations I do have the need to
Ability to decorate ChangeListener
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
