fixing bug in FilterListIterator, adding test for it

javadoc fixes to FilterListIterator


git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/collections/trunk@130599 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/commons/collections/FilterListIterator.java

@@ -1,7 +1,7 @@
 /*
- * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/java/org/apache/commons/collections/Attic/FilterListIterator.java,v 1.1 2002/02/25 23:53:20 rwaldhoff Exp $
+ * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/java/org/apache/commons/collections/Attic/FilterListIterator.java,v 1.2 2002/02/26 17:28:55 rwaldhoff Exp $
- * $Revision: 1.1 $
+ * $Revision: 1.2 $
- * $Date: 2002/02/25 23:53:20 $
+ * $Date: 2002/02/26 17:28:55 $
  *
  * ====================================================================
  *
@@ -64,19 +64,21 @@ import java.util.ListIterator;
 import java.util.NoSuchElementException;
 
 /** 
- * A proxy {@link ListIterator ListIterator} which takes a {@link Predicate Predicate} instance to filter
+  * A proxy {@link ListIterator ListIterator} which 
-  * out objects from an underlying {@link Iterator Iterator} instance.
+  * takes a {@link Predicate Predicate} instance to filter
-  * Only objects for which the
+  * out objects from an underlying <code>ListIterator</code> 
-  * specified <code>Predicate</code> evaluates to <code>true</code> are
+  * instance. Only objects for which the specified 
-  * returned.
+  * <code>Predicate</code> evaluates to <code>true</code> are
+  * returned by the iterator.
   * 
-  * @author <a href="mailto:jstrachan@apache.org">James Strachan</a>
+  * @version $Revision: 1.2 $ $Date: 2002/02/26 17:28:55 $
-  * @author Jan Sorensen
+  * @author Rodney Waldhoff
   */
-
 public class FilterListIterator extends ProxyListIterator {
 
+    // Constructors    
     //-------------------------------------------------------------------------
+    
     public FilterListIterator() {
     }
 
@@ -89,9 +91,14 @@ public class FilterListIterator extends ProxyListIterator {
         this.predicate = predicate;
     }
 
+    public FilterListIterator(Predicate predicate) {
+        this.predicate = predicate;
+    }
+
     // ListIterator interface
     //-------------------------------------------------------------------------
 
+    /** Not supported. */
     public void add(Object o) {
         throw new UnsupportedOperationException("FilterListIterator.add(Object) is not supported.");
     }
@@ -118,9 +125,10 @@ public class FilterListIterator extends ProxyListIterator {
                 throw new NoSuchElementException();
             }
         }
-        nextObjectSet = false;
         nextIndex++;
-        return nextObject;
+        Object temp = nextObject;
+        clearNextObject();
+        return temp;
     }
 
     public int nextIndex() {
@@ -133,19 +141,22 @@ public class FilterListIterator extends ProxyListIterator {
                 throw new NoSuchElementException();
             }
         }
-        previousObjectSet = false;
         nextIndex--;
-        return previousObject;
+        Object temp = previousObject;
+        clearPreviousObject();
+        return temp;
     }
 
     public int previousIndex() {
         return (nextIndex-1);
     }
 
+    /** Not supported. */
     public void remove() {
         throw new UnsupportedOperationException("FilterListIterator.remove() is not supported.");
     }
 
+    /** Not supported. */
     public void set(Object o) {
         throw new UnsupportedOperationException("FilterListIterator.set(Object) is not supported.");
     }
@@ -168,14 +179,29 @@ public class FilterListIterator extends ProxyListIterator {
         this.predicate = predicate;
     }
 
-    /**
+    // Private Methods
-     * Set {@link #nextObject} to the next object. If there 
+    //-------------------------------------------------------------------------
-     * are no more objects then return <code>false</code>. 
+
-     * Otherwise, return <code>true</code>.
+    private void clearNextObject() {
-     */
+        nextObject = null;
+        nextObjectSet = false;
+    }
+
     private boolean setNextObject() {
         ListIterator iterator = getListIterator();
         Predicate predicate = getPredicate();
+        
+        // if previousObjectSet,
+        // then we've walked back one step in the 
+        // underlying list (due to a hasPrevious() call)
+        // so skip ahead one matching object
+        if(previousObjectSet) {
+            clearPreviousObject();
+            if(!setNextObject()) {
+                return false;
+            }
+        }
+
         while(iterator.hasNext()) {
             Object object = iterator.next();
             if(predicate.evaluate(object)) {
@@ -187,14 +213,26 @@ public class FilterListIterator extends ProxyListIterator {
         return false;
     }
 
-    /**
+    private void clearPreviousObject() {
-     * Set {@link #nextObject} to the next object. If there 
+        previousObject = null;
-     * are no more objects then return <code>false</code>. 
+        previousObjectSet = false;
-     * Otherwise, return <code>true</code>.
+    }
-     */
+
     private boolean setPreviousObject() {
         ListIterator iterator = getListIterator();
         Predicate predicate = getPredicate();
+        
+        // if nextObjectSet,
+        // then we've walked back one step in the 
+        // underlying list (due to a hasNext() call)
+        // so skip ahead one matching object
+        if(nextObjectSet) {
+            clearNextObject();
+            if(!setPreviousObject()) {
+                return false;
+            }
+        }
+
         while(iterator.hasPrevious()) {
             Object object = iterator.previous();
             if(predicate.evaluate(object)) {
@@ -224,7 +262,6 @@ public class FilterListIterator extends ProxyListIterator {
      */
     private boolean nextObjectSet = false;   
 
-
     /** 
      * The value of the previous (matching) object, when 
      * {@link #previousObjectSet} is true. 

src/test/org/apache/commons/collections/TestFilterListIterator.java

@@ -1,7 +1,7 @@
 /*
- * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/Attic/TestFilterListIterator.java,v 1.1 2002/02/25 23:53:20 rwaldhoff Exp $
+ * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/Attic/TestFilterListIterator.java,v 1.2 2002/02/26 17:28:55 rwaldhoff Exp $
- * $Revision: 1.1 $
+ * $Revision: 1.2 $
- * $Date: 2002/02/25 23:53:20 $
+ * $Date: 2002/02/26 17:28:55 $
  *
  * ====================================================================
  *
@@ -70,7 +70,7 @@ import java.util.ListIterator;
 import java.util.Random;
 
 /**
- * @version $Revision: 1.1 $ $Date: 2002/02/25 23:53:20 $
+ * @version $Revision: 1.2 $ $Date: 2002/02/26 17:28:55 $
  * @author Rodney Waldhoff
  */
 public class TestFilterListIterator extends TestCase {
@@ -112,6 +112,7 @@ public class TestFilterListIterator extends TestCase {
             list.add(new Integer(i));
             if(i%2 == 0) { evens.add(new Integer(i)); }
             if(i%2 == 1) { odds.add(new Integer(i)); }
+            if(i%3 == 0) { threes.add(new Integer(i)); }
             if(i%4 == 0) { fours.add(new Integer(i)); }
             if(i%6 == 0) { sixes.add(new Integer(i)); }
         }
@@ -280,26 +281,98 @@ public class TestFilterListIterator extends TestCase {
         walkLists(sixes,filtered);
     }
 
+    public void testNestedSixes3() {        
+        FilterListIterator filtered = new FilterListIterator(
+                                        new FilterListIterator(list.listIterator(),threePred),
+                                        evenPred
+                                      );
+        walkLists(sixes,new FilterListIterator(filtered,truePred));
+    }
+
+    public void testNextChangesPrevious() {
+        {
+            FilterListIterator filtered = new FilterListIterator(list.listIterator(),threePred);
+            nextNextPrevious(threes.listIterator(),filtered);
+        }
+    
+        {
+            FilterListIterator filtered = new FilterListIterator(list.listIterator(),truePred);
+            nextNextPrevious(list.listIterator(),filtered);
+        }
+    }
+
+    public void testPreviousChangesNext() {
+        {
+            FilterListIterator filtered = new FilterListIterator(list.listIterator(),threePred);
+            ListIterator expected = threes.listIterator();
+            walkForward(expected,filtered);
+            previousPreviousNext(expected,filtered);
+        }
+        {
+            FilterListIterator filtered = new FilterListIterator(list.listIterator(),truePred);
+            ListIterator expected = list.listIterator();
+            walkForward(expected,filtered);
+            previousPreviousNext(expected,filtered);
+        }
+    }
+
     // Utilities
 
+    private void walkForward(ListIterator expected, ListIterator testing) {
+        while(expected.hasNext()) {
+            assertEquals(expected.nextIndex(),testing.nextIndex());
+            assertEquals(expected.previousIndex(),testing.previousIndex());
+            assertTrue(testing.hasNext());
+            assertEquals(expected.next(),testing.next());
+        }
+    }
+
+    private void walkBackward(ListIterator expected, ListIterator testing) {
+        while(expected.hasPrevious()) {
+            assertEquals(expected.nextIndex(),testing.nextIndex());
+            assertEquals(expected.previousIndex(),testing.previousIndex());
+            assertTrue(testing.hasPrevious());
+            assertEquals(expected.previous(),testing.previous());
+        }
+    }
+
+    private void nextNextPrevious(ListIterator expected, ListIterator testing) {
+        // calls to next() should change the value returned by previous()
+        // even after previous() has been set by a call to hasPrevious()
+        assertEquals(expected.next(),testing.next());
+        assertEquals(expected.hasPrevious(),testing.hasPrevious());
+        Object expecteda = expected.next();
+        Object testinga = testing.next();
+        assertEquals(expecteda,testinga);
+        Object expectedb = expected.previous();
+        Object testingb = testing.previous();
+        assertEquals(expecteda,expectedb);
+        assertEquals(testinga,testingb);
+    }
+
+    private void previousPreviousNext(ListIterator expected, ListIterator testing) {
+        // calls to previous() should change the value returned by next()
+        // even after next() has been set by a call to hasNext()
+        assertEquals(expected.previous(),testing.previous());
+        assertEquals(expected.hasNext(),testing.hasNext());
+        Object expecteda = expected.previous();
+        Object testinga = testing.previous();
+        assertEquals(expecteda,testinga);
+        Object expectedb = expected.next();
+        Object testingb = testing.next();
+        assertEquals(expecteda,testingb);
+        assertEquals(expecteda,expectedb);
+        assertEquals(testinga,testingb);
+    }
+
     private void walkLists(List list, ListIterator testing) {
         ListIterator expected = list.listIterator();
 
         // walk all the way forward
-        while(expected.hasNext()) {
+        walkForward(expected,testing);
-            assertEquals(expected.nextIndex(),testing.nextIndex());
-            assertEquals(expected.previousIndex(),testing.previousIndex());
-            assertTrue("a",testing.hasNext());
-            assertEquals("b",expected.next(),testing.next());
-        }
 
         // walk all the way back
-        while(expected.hasPrevious()) {
+        walkBackward(expected,testing);
-            assertEquals(expected.nextIndex(),testing.nextIndex());
-            assertEquals(expected.previousIndex(),testing.previousIndex());
-            assertTrue("c",testing.hasPrevious());
-            assertEquals("d",expected.previous(),testing.previous());
-        }
 
         // forward,back,foward
         while(expected.hasNext()) {
@@ -313,13 +386,9 @@ public class TestFilterListIterator extends TestCase {
             assertEquals(expected.next(),testing.next());
         }
 
+
         // walk all the way back
-        while(expected.hasPrevious()) {
+        walkBackward(expected,testing);
-            assertEquals(expected.nextIndex(),testing.nextIndex());
-            assertEquals(expected.previousIndex(),testing.previousIndex());
-            assertTrue(testing.hasPrevious());
-            assertEquals(expected.previous(),testing.previous());
-        }
 
         for(int i=0;i<list.size();i++) {
             // walk forward i