[COLLECTIONS-447] Fix traversal of a TreeListIterator after calling remove(). Thanks to Jeffrey Barnes for the report and patch.

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/collections/trunk@1452481 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Thomas Neidhart 2013-03-04 20:20:18 +00:00
parent 89ebfe8f92
commit b88692f83a
3 changed files with 30 additions and 6 deletions

View File

@ -22,6 +22,10 @@
<body> <body>
<release version="4.0" date="TBA" description="Next release"> <release version="4.0" date="TBA" description="Next release">
<action issue="COLLECTIONS-447" dev="tn" type="fix" due-to="Jeffrey Barnes">
Tree traversal with a TreeListIterator will not be affected anymore by
the removal of an element directly after a call to previous().
</action>
<action issue="COLLECTIONS-275" dev="tn" type="add" due-to="Stephen Kestle"> <action issue="COLLECTIONS-275" dev="tn" type="add" due-to="Stephen Kestle">
Added "IndexedCollection" collection decorator which provides a map-like Added "IndexedCollection" collection decorator which provides a map-like
view on an existing collection. view on an existing collection.

View File

@ -900,15 +900,14 @@ public class TreeList<E> extends AbstractList<E> {
if (currentIndex == -1) { if (currentIndex == -1) {
throw new IllegalStateException(); throw new IllegalStateException();
} }
if (nextIndex == currentIndex) { parent.remove(currentIndex);
// remove() following previous() if (nextIndex != currentIndex) {
next = next.next();
parent.remove(currentIndex);
} else {
// remove() following next() // remove() following next()
parent.remove(currentIndex);
nextIndex--; nextIndex--;
} }
// the AVL node referenced by next may have become stale after a remove
// reset it now: will be retrieved by next call to next()/previous() via nextIndex
next = null;
current = null; current = null;
currentIndex = -1; currentIndex = -1;
expectedModCount++; expectedModCount++;

View File

@ -46,6 +46,7 @@ public class TreeListTest<E> extends AbstractListTest<E> {
// benchmark(new java.util.ArrayList()); // benchmark(new java.util.ArrayList());
// System.out.print("\n LinkedList = "); // System.out.print("\n LinkedList = ");
// benchmark(new java.util.LinkedList()); // benchmark(new java.util.LinkedList());
// System.out.print("\n NodeCachingLinkedList = ");
// benchmark(new NodeCachingLinkedList()); // benchmark(new NodeCachingLinkedList());
// } // }
@ -246,5 +247,25 @@ public class TreeListTest<E> extends AbstractListTest<E> {
assertEquals(new Integer(4), li.next()); assertEquals(new Integer(4), li.next());
assertEquals(false, li.hasNext()); assertEquals(false, li.hasNext());
} }
public void testBugCollections447() {
final List<String> treeList = new TreeList<String>();
treeList.add("A");
treeList.add("B");
treeList.add("C");
treeList.add("D");
final ListIterator<String> li = treeList.listIterator();
assertEquals("A", li.next());
assertEquals("B", li.next());
assertEquals("B", li.previous());
li.remove(); // Deletes "B"
// previous() after remove() should move to
// the element before the one just removed
assertEquals("A", li.previous());
}
} }