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 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>
> 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
>
>
