On Sat, 24 Apr 2021 18:29:47 GMT, Kevin Rushforth <[email protected]> wrote:
> Also, if you could explain the logic changes a bit, that would be helpful.
Here's my guess what the original implementation tried to accomplish:
...
while (endPos < removed.size()) {
if (removed.get(startPos) == removed.get(endPos) - 1) {
endPos++;
continue;
}
...
}
`removed` is an ascending list of indices that should be removed from
`selectedIndices`. The loop tries to find uninterrupted ascending ranges within
that list (for example, `[2, 3], [5, 6, 7]`). However, the condition
`removed.get(startPos) == removed.get(endPos) - 1` will always fail after just
a single iteration, since `startPos` is not modified before `continue`, which
means that the loop will never detect any range longer than 2 elements.
The second issue with the original code is at this line:
selectedIndices._nextRemove(selectedIndices.indexOf(removed.get(startPos)),
removed.subList(startPos, endPos));
The first parameter of `_nextRemove` is the index at which the removal
happened, and this index must match the current state of the list.
Let's assume that `selectedIndices` contains `[1, 2, 3, 4, 5, 6, 7, 8]`, and
`removed` contains `[2, 3, 5, 6, 7]`.
In this case, the first removed range is `[2, 3]`, which happens at index `1`.
The second removal must account for the already-removed elements: `[5, 6, 7]`
at index `2`.
Here's the new implementation (see the comments):
for (int endPos = 0, max = removed.size(); endPos < max; ++endPos) {
// This loop increments endPos as long as the next index is adjacent to
the previous index
while (endPos < max - 1 && removed.get(endPos) == removed.get(endPos +
1) - 1) {
++endPos;
}
// totalRemoved accounts for the elements which have already been
removed, so that the
// index will refer to the state of the list after all previous removal
changes have been applied
selectedIndices._nextRemove(
selectedIndices.indexOf(removed.get(startPos)) - totalRemoved,
removed.subList(startPos, endPos + 1));
totalRemoved += endPos - startPos + 1;
startPos = endPos + 1;
}
-------------
PR: https://git.openjdk.java.net/jfx/pull/480
