On Tue, 29 Sep 2026 22:52:58 GMT, Kevin Rushforth <[email protected]> wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update documentation according to review comments
>
> modules/javafx.base/src/main/java/javafx/beans/value/ObservableListValue.java
> line 35:
>
>> 33: * @implNote
>> 34: * The implementations of this interface in the JavaFX library do not
>> provide all
>> 35: * of the guarantees described by {@link ObservableValue} for change
>> listeners: the
>
> In addition to the ones listed below, can you add that equal values may
> trigger a notification? That guarantee from [ObservableValue lines
> 97-99](https://github.com/hjohn/jfx/blob/871cbc31695403dd2f9bfaaaa5353478a0d8fb33/modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java#L97-L99)
> doesn't hold. This also applies to the observable map and set value
> interfaces.
On the `equals` point, I think it's the same underlying issue as @mstr2 raised
above:
> I think that invoking a change listener when only the contents of a list has
> changed is a jarring defect. A change listener should only be invoked when
> the list instance is changed; for its contents, we have ListChangeListener.
> Now, I'm not sure if we can ever fix this, but maybe we shouldn't enshrine
> this defect into specification with this PR. If we come to the conclusion
> that it can't be changed, we can always specify this behavior later.
I think this may be a longer discussion, as there are several issues with how
change listeners work for these that deviate strongly from how they work for
properties:
| Case | Property / binding `ChangeListener` | `ObservableListValue`
`ChangeListener` |
|---|---|---|
| `set(x)` where `x == old` (same instance) | no notification (`set` no-op) |
no notification (`set` no-op) |
| `set(x)` where `x != old` but `x.equals(old)` | **no notification** (the
manager suppresses equal values) | **notification**, with `old.equals(new)` and
`old != new` |
| `set(x)` where `x != old` and `!x.equals(old)` | notification (`old`, `new`)
| notification (`old`, `new`) |
| contents change (same list instance) | n/a | **notification**, with `old ==
new` (identical instance) |
These are understandable choices, but it may have been better not to reuse the
same `ChangeListener` interface given how they deviate (especially a reference
change, but still being equal). I'd therefore hold off on specifying this
behaviour for now, as @mstr2 suggested -- **however** I think it truly can't be
fixed; using a change listener for lists to indicate reference changes is
exactly what property listeners **don't** do -- so I think I still stand by my
stance that they should be deprecated for the collection value types and one
should use `InvalidationListener` + `getValue()` if they want the old
`ChangeListener` behavior, or use the true API: `ListChangeListener`.
Going a bit wider, this is worth getting right because change listeners (for
properties) intentionally don't notify when `equals()` (which seems sensible),
but that does open you up for "large object is replaced with another large but
`equals()` object" and holding on to the old reference since a change listener
doesn't get notified then. For `String`s this is already the case, but likely
deemed acceptable since `String`s are often interned, but strictly speaking, a
change listener that is keeping a reference to a received new value may be
holding on to duplicate `equals()` data. So if you really care about the
**exact** reference/instance (and not pinning a duplicate object in memory if
you keep a reference), you must use an `InvalidationListener`.
That in turn relies on a few assumptions we're already making on property
values:
- The value must not be mutated in place; changes must be made by replacing it
via `set`; in other words, the value must be immutable for as long as the
property or a listener holds it
- The value must have a stable, meaningful `equals()` implementation
- Setting a value that is `equals()` to the current one is not a change: for
`ObjectProperty` it still invalidates (so invalidation listeners run) but
produces no change event; for `StringProperty` it does nothing at all
- `ChangeListener` must therefore not be used to observe the exact instance;
instead use `InvalidationListener`, since the change listener's `newValue` may
be an earlier, `equals()` instance rather than the current one
The value properties and `String` all match those criteria, and for most object
properties in JavaFX we use immutable values (i.e. `Color`, `Insets`, `Border`,
etc.)
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r4177186959
