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
modules/javafx.base/src/main/java/com/sun/javafx/collections/ObservableSetWrapper.java
line 443:
> 441: backingSet.toArray(removed);
> 442: backingSet.clear();
> 443: callObservers(new IterableSetChange.Remove<>(this,
> Arrays.asList(removed)));
suggestion:
ArrayList<E> removed = new ArrayList<>(backingSet);
backingSet.clear();
callObservers(new IterableSetChange.Remove<>(this, removed));
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1885#discussion_r2461639599
