On Sat, 3 Oct 2020 11:13:28 GMT, Jeanette Winzenburg <faste...@openjdk.org>
wrote:
> The listView test is passing for the bitSet and for the back-to-front
> approach. Can we imagine a context where the
> broken selectedItems impl would add wreckage to the latter?
To answer my own question: yes. A failing test with the back-to-front approach
(the existing test in ListViewTest
modified in having the last item selected)
@Test public void test_rt35857Back() {
ObservableList<String> fxList = FXCollections.observableArrayList("A",
"B", "C");
final ListView<String> listView = new ListView<String>(fxList);
listView.getSelectionModel().select(2);
ObservableList<String> selectedItems =
listView.getSelectionModel().getSelectedItems();
assertEquals(1, selectedItems.size());
assertEquals("C", selectedItems.get(0));
listView.getItems().removeAll(selectedItems);
assertEquals(2, fxList.size());
assertEquals("A", fxList.get(0));
assertEquals("B", fxList.get(1));
}
Feels like coming nearer to the why of the BitSet approach: it guards against
the scenario where changing the current
list implicitly changes the list given as parameter - in that case, it's unsafe
to query the parameter list while
removing items (c.contains simply reports nonsense).
This may happen whenever the parameter list does some kind of live lookup into
the current list (such as f.i.
SelectedItemsReadOnlyObservableList) - which is not as evil as I thought
yesterday (did it myself in custom
selectionModels ;) The BitSet solves it by a two-pass approach: first collect
all items to remove/retain without (that
is keep the parameter list in a valid state, allowing to safely access it),
then do the removal without further
accessing the (then invalid) parameter list.
The fix at that time was a deliberate decision by the designer of the
collections, so the context when it happens was
deemed a valid use-case. Looks like we should **not** revert it.
-------------
PR: https://git.openjdk.java.net/jfx/pull/305
