On 03/10/2013 16:31, Brent Christian wrote:
Please review my fix for 8024709 : "TreeMap.DescendingKeyIterator
'remove' confuses iterator position"
There are two possible code paths for performing a "descending"
iteration over the elements in a TreeMep, depending on how the
iteration is setup.
For instance,
treemap.descendingKeySet().iterator();
will use a
TreeMap.NavigableSubMap.DescendingSubMapIterator
This code correctly handles Iterator.remove().
On the other hand,
treemap.navigableKeySet().descendingIterator();
will use a
TreeMap.DescendingKeyIterator.
This code does not correctly handle remove(), and results in a
"confused" iterator.
TreeMap.DescendingKeyIterator should override remove() with code
similar to that in
TreeMap.NavigableSubMap.SubMapIterator.removeDescending().
Bug report:
https://bugs.openjdk.java.net/browse/JDK-8024709
Webrev:
http://cr.openjdk.java.net/~bchristi/8024709/webrev.00/
The remove looks right to me and next() should return the predecessor
that is already set.
The test looks okay (although you might want to align the parameters to
checkDescendingIteratorRemove), I just wonder if you would be worth
generating additional cases to exercise this code a bit more.
-Alan
