On Thu, 23 Oct 2025 23:31:01 GMT, Michael Strauß <[email protected]> wrote:
>> While a `ListChangeListener` can receive notifications for bulk operations
>> (`addAll`, `removeAll`, `clear`, etc.), `SetChangeListener` and
>> `MapChangeListener` only receive notifications for individual
>> add/replace/delete operations. For example, when mappings are added to an
>> `ObservableMap` with `putAll()`, listeners will be invoked once for each
>> individual mapping.
>>
>> Since there is no way for a `SetChangeListener`/`MapChangeListener` to know
>> that more changes are coming, reacting to changes becomes difficult and
>> potentially inefficient if an expensive operation (like reconfiguring the
>> UI) is done for each individual change instead of once for a bulk change
>> operation.
>>
>> I think we can improve the situation by adding a new method to
>> `SetChangeListener.Change` and `MapChangeListener.Change`:
>>
>>
>> /**
>> * Gets the next change in a series of changes.
>> * <p>
>> * Repeatedly calling this method allows a listener to fetch all subsequent
>> changes of a bulk
>> * map modification that would otherwise be reported as repeated invocations
>> of the listener.
>> * If the listener only fetches some of the pending changes, the rest of the
>> changes will be
>> * reported with subsequent listener invocations.
>> * <p>
>> * After this method has been called, the current {@code Change} instance is
>> no longer valid and
>> * calling any method on it may result in undefined behavior. Callers must
>> not make any assumptions
>> * about the identity of the {@code Change} instance returned by this
>> method; even if the returned
>> * instance is the same as the current instance, it must be treated as a
>> distinct change.
>> *
>> * @return the next change, or {@code null} if there are no more changes
>> */
>> public Change<E> next() { return null; }
>>
>>
>> This new method allows listener implementations to fetch all subsequent
>> changes of a bulk operation, which can be implemented as follows:
>>
>>
>> set.addListener((SetChangeListener) change -> {
>> do {
>> // Inspect the change
>> if (change.wasAdded()) {
>> ...
>> } else if (change.wasRemoved() {
>> ...
>> }
>> } while ((change = change.next()) != null);
>> }
>>
>>
>> The implementation is fully backwards-compatible for listeners that are
>> unaware of the new API. If the `next()` method is not called, then all
>> subsequent changes are delivered as usual by repeated listener invocations.
>>
>> If a listener only fetches some changes of a bulk operation (but stops
>> halfway through the op...
>
> Michael Strauß has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains eight additional
> commits since the last revision:
>
> - Merge branch 'master' into feature/bulk-listeners
> - remove unused variable
> - Don't repeatedly call backingSet.size()
> - Separate code paths for Change/IterableChange
> - Use MapListenerHelper in PlatformPreferences to support bulk change
> notifications
> - Factor out IterableSetChange/IterableMapChange implementations
> - add tests, documentation
> - Implementation of bulk change listeners for ObservableSet and ObservableMap
There's one behavioral change with this enhancement that I want to point out.
Previously, bulk operations were just repeated invocations of their
single-element counterpart, i.e. `ObservableSetWrapper.addAll(Collection<E>)`
would just repeatedly call `ObservableSetWrapper.add(E)` and thereby trigger
repeated listener invocations.
Now consider this method:
public interface SetChangeListener<E> {
abstract class Change<E> {
/**
* An observable set that is associated with the change.
* @return the source set
*/
public ObservableSet<E> getSet() { ... }
}
}
Previously, if a listener were to query the source set, it would always
correspond to the _current_ state of the set.
With the bulk change enhancement, the source set corresponds to the _final_
state of the set (because all changes have already been applied in bulk).
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1885#issuecomment-3440077428
