On Wed, 15 Oct 2025 12:08:36 GMT, Michael Strauß <[email protected]> wrote:

>> This PR implements two new default methods on `ObservableList` to be able to 
>> replace elements at a given position or 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.
>
> modules/javafx.base/src/main/java/javafx/collections/ObservableList.java line 
> 92:
> 
>> 90:      *
>> 91:      * @param index the start index at which to replace elements, must 
>> be in
>> 92:      *              {@code 0 .. size() - col.size()}
> 
> This seems to be an unnecessary limitation. It means that as a result of this 
> operation, ~~the list can only be shorter, but not longer~~ the size of the 
> list must stay the same. The `setAll(int, int, Collection)` overload doesn't 
> have this limitation.

Initially, I only wanted to support a "pure" `set` function (to avoid a 
`set(int)` loop + notifications), where you can have a range of elements 
replaced by another range of the exact same size (so an `int` and `Collection` 
is enough to define this).  However, there was an open ticket to have a variant 
`(from, to, Collection)` which is more powerful, but is not truly a "set" 
function anymore IMHO.

I figured I could implement both, but I think having only `set(int, 
Collection)` would be sufficient for most use cases.

I suppose it is possible to relax this, but it would have a bit of weird usage 
I think:

     List.of(A, B, C, D).setAll(1, List.of(E, F, G))  -> A, E, F, G
     List.of(A, B, C, D).setAll(2, List.of(E, F, G))  -> A, B, E, F, G
     List.of(A, B, C, D).setAll(3, List.of(E, F, G))  -> A, B, C, E, F, G
     
Now it can also extend the collection, which looks weird.  The other 
`setAll(from, to, col)` does indeed allow this, but perhaps it shouldn't be 
called `setAll` as it is not strictly doing what a loop of `set` calls normally 
could be doing...

Perhaps I shouldn't have attempted to reuse an old ticket, and just keep it 
simple with only a `setAll(int, Collection)`...

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1937#discussion_r2432401418

Reply via email to