RFR: 8233179: VetoableListDecorator#sort throws IllegalArgumentException "duplicate children"
Kevin Rushforth
kcr at openjdk.org
Tue Feb 25 16:30:05 UTC 2025
On Tue, 14 Jan 2025 00:55:36 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:
> The `VetoableListDecorator` class does not override the default implementation of `List.sort()`. The default implementation copies the elements into an array, sorts the array, and then calls `List.set()` in a loop to sort the List **without removing the old elements first**. This leads to the `VetoableListDecorator` intercepting the call to `set()` and seeing a "duplicate child" being added.
>
> The solution is to implement `SortableList.doSort()` with the following protocol:
> 1. Make a copy of the list and sort it.
> 2. Present the sorted list with `onProposedChange` for a potential veto.
> 3. If successful, use `setAll()` to swap out the current list with the sorted list.
LGTM. I left one question and will re-review if you decide to change.
modules/javafx.base/src/main/java/com/sun/javafx/collections/VetoableListDecorator.java line 115:
> 113: modCount--;
> 114: throw e;
> 115: }
Could this be replaced by `setAll(sortedList)`?
-------------
Marked as reviewed by kcr (Lead).
PR Review: https://git.openjdk.org/jfx/pull/1674#pullrequestreview-2641703891
PR Review Comment: https://git.openjdk.org/jfx/pull/1674#discussion_r1970119348
More information about the openjfx-dev
mailing list