ListProperty::bind has different behavior than StringProperty::bind when observables are equal

[José Pereda]

Thanks, that's helpful.

If I get it right, the change applied in ListExpressionHelper [1][2]
removed equals:

-                if ((currentValue == null)? (oldValue != null) :
!currentValue.equals(oldValue)) {
+                if (currentValue != oldValue) {

So that will imply that we should do the same in ListPropertyBase::bind?


[1] http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/26b7aedb6d60
[2]
https://github.com/openjdk/jfx/blob/22d4343fe8563c2931910b98e8f18c6fd4a48f05/modules/javafx.base/src/main/java/com/sun/javafx/binding/ListExpressionHelper.java#L552

On Tue, Jun 9, 2020 at 2:33 PM Jeanette Winzenburg <fastegal at swingempire.de>
wrote:

>
> sounds a bit like https://bugs.openjdk.java.net/browse/JDK-8094799 -
> listProperty was fixed for notification of changeListeners to test
> against identity always, should do in binding as well, IMO
>
> Zitat von José Pereda <jose.pereda at gluonhq.com>:
>
> > Hi,
> >
> > We have come across a possible issue that affects bind() over different
> > types of properties.
> > We are not sure if it's a bug or it behaves as it should be. Therefore,
> we
> > are reaching out to the community for insight.
> >
> > See these short code snippets below:
> >
> > Snippet 1. StringProperty (could be other like IntegerProperty)
> >
> > StringProperty a = new SimpleStringProperty();
> > StringProperty b = new SimpleStringProperty();
> > StringProperty c = new SimpleStringProperty();
> > c.addListener((obs, ov, nv) -> System.out.println("Change: " + nv));
> > c.bind(a);
> > c.bind(b);
> > b.set("One");
> > System.out.println("c contains: " + c.get());
> > ====
> >
> > Snippet 2. ListProperty
> >
> > ListProperty<String> a = new
> > SimpleListProperty<>(FXCollections.<String>observableArrayList());
> > ListProperty<String> b = new
> > SimpleListProperty<>(FXCollections.<String>observableArrayList());
> > ListProperty<String> c = new SimpleListProperty<>();
> > c.addListener((ListChangeListener.Change<? extends String> change) ->
> > System.out.println("Change: " + change));
> > c.bind(a);
> > c.bind(b);
> > b.add("One");
> > b.add("Two");
> > System.out.println("c contains: " + Arrays.toString(c.toArray()));
> > ====
> >
> > While snippet 1 produces:
> >
> > Change: One
> > c contains: One
> >
> > as expected, snippet 2 produces:
> >
> > Change: { [] added at 0 }
> > c contains: []
> >
> > But the expected result is:
> >
> > Change: { [] added at 0 }
> > Change: { [] added at 0 }
> > Change: { [One] added at 0 }
> > Change: { [Two] added at 1 }
> > c contains: [One, Two]
> >
> > This expected result only happens by manually doing:
> >
> > c.bind(a);
> > c.unbind(); // need to explicitly unbind
> > c.bind(b);
> >
> > Summing up: when binding a new observable to a bound property, unbind is
> > required if that property is a ListProperty, but not a StringProperty (or
> > IntegerProperty).
> >
> > Explanation
> >
> > In the ListProperty.bind implementation [1] there is a check, and only
> when
> > both current bound observable and new observable are equal, unbind() is
> > applied.
> >
> > The equals is implemented in ReadOnlyListProperty::equals [2] and, being
> a
> > list, follows the List::equals[3] contract, therefore, it compares by
> items
> > of the list.
> >
> > This is not the case when using other properties like StringPropertyBase
> or
> > IntegerPropertyBase, where bind() is implemented in the same way [4], but
> > equals now uses Object::equals[5], so the comparison is now done with ==,
> > instead of comparing by the content of each property.
> >
> > Is this a bug? Should StringProperty, IntegerProperty override equals to
> > compare by content and not by object, like ListProperty does? Should
> > ListProperty use == operator instead of equals?
> >
> > This difference in behavior should be at least documented, shouldn't it?
> >
> >
> > Thanks for any input on this,
> > Jose
> >
> >
> > [1]
> >
> https://github.com/openjdk/jfx/blob/22d4343fe8563c2931910b98e8f18c6fd4a48f05/modules/javafx.base/src/main/java/javafx/beans/property/ListPropertyBase.java#L268
> > [2]
> >
> https://github.com/openjdk/jfx/blob/22d4343fe8563c2931910b98e8f18c6fd4a48f05/modules/javafx.base/src/main/java/javafx/beans/property/ReadOnlyListProperty.java#L110
> > [3]
> >
> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/List.java#L542
> > [4]
> >
> https://github.com/openjdk/jfx/blob/22d4343fe8563c2931910b98e8f18c6fd4a48f05/modules/javafx.base/src/main/java/javafx/beans/property/StringPropertyBase.java#L161
> > [5]
> >
> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/Object.java#L150
> >
> > --
>
>
>
>

--