Hi list,
I've been busy trying to ensure that ChangeListeners received sensible
old/new values at all times. This is important as users may be relying
on these values to be correct when doing calculations or creating nested
bindings (where an old listener is removed based on the old value
received, and a new listener is registered based on the new value
received).
As you may know, currently the values received by ChangeListeners are
not what you'd expect when the value is modified within an ongoing
notification (a nested change). This affects ChangeListeners registered
*after* the listener that triggered the nested change. These listeners
receive incorrect old values, and duplicate new values. It gets even
more complicated when multiple listeners do this or when listeners are
added/removed during the notification process (all of which is allowed).
I would like to update the ChangeListener javadoc to document what the
user can expect when they register their listener. The current
documentation implies that:
- The listener is only called when a change occurs (implying new value
received changed from last time)
- The above further implies old and new values are distinct as it must
have changed
- The fact that it is called the "old value" implies that it will hold
the value of the previously received new value and not some other value
that is distinct from the received new value
If we can agree on the above, we may want to specify this contract a bit
more explicitly.
Also I'd like to explicitly state that the received new value need not
be the same as the value obtained from `ObservableValue#getValue` when
another notification is still pending.
A further rule we should discuss is whether all ChangeListeners should
receive notifications about **all** changes to a property. The current
implementation will notify all listeners X times when there were X
changes, but, when there are nested changes, will not actually send all
the changes as a "new value" (it skips some of the values, and sends
some twice). I see a few options here:
1. All ChangeListeners receive notifications about all changes exactly
as they occurred, regardless of nested changes made by listeners earlier
in the chain of listeners.
2. ChangeListeners registered later in the chain of listeners will not
see changes that were changed (vetoed) by earlier listeners. This
implies that if there were X changes, later listeners may see X - Y
changes, where Y is the number of nested changes made. This also implies
there is a strict order in which listeners are registered and called.
I would be inclined to go with option 1 here, but I would like to hear
what others think. The current implementation in my PR
(https://github.com/openjdk/jfx/pull/837) changes `ExpressionHelper` to
follow the above contract, and uses option 1, where all change listeners
receive all notifications exactly as they occurred.
The current implementations in JavaFX breaks the above contract in
several ways:
1) Old value and new value can be the same instance. The collection
properties (ListProperty) do this to avoid having to make a copy of the
collection for a proper old value. I think this okay, but they need to
document they are deviating here from the ChangeListener contract.
Effectively, it is no longer a change listener, it only provides the
current values.
2) New value received is the same new value as the previous call. This
happens when an earlier listener makes a change, triggering a nested
notification. When the nested notification completes, the parent
notification continues by sending the same new value again to the later
listeners.
3) Old value received is not the same as the new value of the previous
call. This happens in the same situation as 2)
I'm hoping we can get this situation fixed as they may trigger subtle
bugs in calculations involving old/new values. When manually tracking
bindings for example it can result in listeners that are never removed
(for incorrect old values) or listeners being registered multiple times
(for duplicate new values).
Thanks for reading!
--John
