RFR: 8091429: ObservableList<E>#setAll(int from, int to, Collection<? extends E> col) [v2]
John Hendrikx
jhendrikx at openjdk.org
Mon Oct 20 05:06:21 UTC 2025
On Wed, 15 Oct 2025 15:09:02 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:
>> I was restricting it because "set" does not allow you to add an element either when giving `size()` as the index. I only belatedly realized that `setAll(int, int, Collection)` is violating this, and allows to add elements and so changes the list size.
>>
>> Naming is really important, so calling the range method `setAll` then is I think a poor choice. So the possible methods we could introduce are:
>>
>> - `setAll(int, Collection)` and disallow it to change size; this method is a combination of `setAll(Collection)` and `set(int, T)` similar to how `addAll(int, Collection)` is a combination of `add(int, T)` and `addAll(Collection)`.
>> - `replaceAll(int, Collection)` similar to the above, but allows adding of elements if it runs out of elements to update
>> - `replaceAll(int, int, Collection)` a proper name for `setAll(int, int, Collection)` (`splice` was also suggested in the ticket).
>>
>> For me I'd either go with:
>> - Have `setAll(int, Collection)`
>> - Have both `setAll(int, Collection)` and `replaceAll(int, int, Collection)`
>>
>> The `replaceAll(int, Collection)` I think does not carry its weight, and would just be confusing given that there would be a much more clear variant `replaceAll(int, int, Collection)`.
>
> * `setAll(int, Collection)` and disallow it to change size
> I think this doesn't carry its weight. It's just an explicit name for a narrow special case of a more general operation. However, if we're going to have a method with this signature, I suggest to not go for the narrow special case, but instead at least allow me to overlap the new list across the end of the existing list.
>
> * `replaceAll(int, int, Collection)`
> We already have `List.replaceAll(UnaryOperator)`, which does something quite different (namely, replacing all elements with different elements). I think your proposed operation really is more like a `replaceRange`.
@mstr2 I was hesitant to introduce another "new" method, and was looking to only overload `setAll`, but thinking it over a bit longer, I think if we want to offer this functionality in a free form way, we may as well just introduce a `replaceRange` method. I removed the `setAll` overload and renamed the range version to `replaceRange`.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1937#discussion_r2443819424
More information about the openjfx-dev
mailing list