On Mon, 20 Oct 2025 05:06:20 GMT, John Hendrikx <[email protected]> wrote:
>> This PR implements a new default method on `ObservableList` to be able to
>> replace elements within a specified range.
>>
>> Justification for this change is to allow an `ObservableList` to be bulk
>> modified resulting in a single `ListChangeListener` call back. In this way
>> the callbacks don't observe the list changing its size from S to S-X back to
>> S again(*). Currently the only way to bulk replace a range of items is to
>> remove X items then add X items, resulting in two listener callbacks in
>> between which the size of the list can be observed to change.
>>
>> The other alternative is to call `set` individually for each item, which
>> results in many change notifications.
>>
>> With the addition of this PR, and the changes in
>> `ModifiableObservableListBase`, replacing a range of items becomes a single
>> change callback.
>>
>> (*) The list may indeed change size still as plain `List` does not have
>> `setAll` operations; size listeners may observe this, but it will no longer
>> be observable from a `ListChangeListener` due to multiple separate callbacks.
>
> John Hendrikx has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Fix test
> - Rename setAll to replaceRange and removed superfluous method
Looks good. I left a question as to whether a couple additional tests might be
useful.
modules/javafx.base/src/test/java/test/javafx/collections/ModifiableObservableListBaseTest.java
line 94:
> 92: @Test
> 93: void shouldReplaceElementsAtGivenRange() {
> 94: assertTrue(list.replaceRange(1, 3, List.of("B", "C", "D")));
Would additional tests be useful?
* Replace a sub-range in the middle (so both beginning and end are kept)
* Replace a sub-range with fewer elements than are removed (the existing test
checks the case of more elements added than removed)
-------------
Marked as reviewed by kcr (Lead).
PR Review: https://git.openjdk.org/jfx/pull/1937#pullrequestreview-3418762471
PR Review Comment: https://git.openjdk.org/jfx/pull/1937#discussion_r2492152669
