Looks like it is even more fuzzy than I thought...
ExpressionHelper (used by almost all property implementations) checks
before sending out changed events using `equals`:
final boolean changed = (currentValue == null)? (oldValue != null)
: !currentValue.equals(oldValue);
What this means is that when you do:
T a = new T();
T b = a.clone();
ObjectProperty<T> x = new SimpleObjectProperty<>(a);
x.set(b);
The above triggers an invalidation because a != b, but no changes
because a.equals(b) even though b here is a totally different instance.
As change listeners weren't called, code may still be referring to a; if
a is mutated, these changes would be seen by such downstream code, while
a `getValue` (which would return b) would not see them.
The behavior is "okay" for immutable T's (of which String is one), but
could still be surprising.
For StringProperty such code would trigger no invalidation and no changes
--John
On 21/03/2023 12:26, John Hendrikx wrote:
I had a run in today with a (known) issue where StringProperty seems
to violate the implementation spec as described by ObservableValue for
which I'd really like to investigate a solution:
* All implementing classes in the JavaFX library check for a change
using reference
* equality (and not object equality, {@code Object#equals(Object)})
of the value.
Yet, in StringPropertyBase#set, we see this:
if ((value == null)? newValue != null :
!value.equals(newValue)) {
value = newValue;
markInvalid();
}
Note the use of `equals` here. In ObjectPropertyBase (and all the
other PropertyBase classes, even something like ListPropertyBase) it's
this:
if (value != newValue) {
value = newValue;
markInvalid();
}
Now, I can understand why this is done, as String is almost regarded
as a primitive class in Java, however, it does make downstream
properties which may not know they're dealing with Strings and that
need to check for changes a bit harder to implement. A downstream
property may want to check for changes to prevent an unnecessary
invalidation for example.
This could look like this:
if (isValid() && source.getValue() != getValue()) {
invalidate();
}
Or this:
T value = source.getValue();
if (cachedValue != value) {
cachedValue = value;
invalidate();
}
Now, since StringProperty strictly speaking violates the
implementation specification for JavaFX provided properties, but not
the general contract of an ObservableValue, there are I think two
possible actions:
1) Make StringProperty no longer violate this implementation
specification and make it use reference equality; this can mean
invalidations and changes are triggered where, according to equals,
the old and new value are the same.
2) Make an exception for String values, as they're almost like
primitives in Java, and update the implementation specification
In the first case, downstream properties can continue to use reference
equality to do their checks. In the second case, we may need to
change these to specifically use `equals` when a String value is
encountered. Strictly speaking that would mean changing
ObjectPropertyBase as well to change its equality:
Old:
if (value != newValue) {
value = newValue;
markInvalid();
}
New:
boolean changed = value instanceof String s ?
!s.equals(newValue) : value != newValue;
if (changed) {
value = newValue;
markInvalid();
}
This way of checking equality would best be encoded somewhere that is
easy to reach for other code that needs to do the same, so I'd propose
a static method on ObservableValue (or somewhere else):
static <T> boolean isEquals(T a, T b) {
return a instanceof String s ? s.equals(b) : a == b;
}
I'd like to hear your thoughts on this as it is can have consequences
for how the `when` fluent binding prevents invalidations, but also
other (future) implementations that sometimes deal with Strings, and
sometimes with any other kind of object.
--John
