commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stephen Colebourne" <scolebou...@btopenworld.com>
Subject Re: [Collections][PATCH] Enable remove in FilterIterator
Date Thu, 30 Jan 2003 23:15:33 GMT
Patch applied, thanks. I had to change TestUniqueFilterIterator too to get
all the tests to pass.

I have applied the patch because it is better behaviour than always throwing
UnsupportedOperationException. The iterator API requires that remove() will
be called after next(). This iterator simply doesn't cover the time after
the following hasNext() is called. But it doesn't fail randomly, it errors,
so there should be no confusion.

Stephen

----- Original Message -----
From: "Shapira, Yoav" <Yoav.Shapira@mpi.com>
Howdy,
This behavior seems a little bit brittle to me.  I'd rather have a
remove() method that is always consistent.  Its function should not
depend on order of operations like where hasNext() has been called or
not.  Unless I'm missing something here?

Yoav Shapira
Millennium ChemInformatics


>-----Original Message-----
>From: Ralph Wagner [mailto:ralph.wagner@web.de]
>Sent: Thursday, January 30, 2003 3:28 AM
>To: commons-dev@jakarta.apache.org
>Subject: [Collections][PATCH] Enable remove in FilterIterator
>
>> At the moment the commons.collections.iterators.FilterIterator does
not
>implement the "remove" method.
>> The comment says:
>>      * Always throws UnsupportedOperationException as this class
>>      * does look-ahead with its internal iterator.
>>
>> This is only a problem if the "hasNext" method was already called,
for
>the
>other cases the remove operation could be implemented.
>>
>> Ralph
>
>Index: FilterIterator.java
>===================================================================
>RCS file: /home/cvspublic/jakarta-
>commons/collections/src/java/org/apache/commons/collections/iterators/F
ilte
>rIterator.java,v
>retrieving revision 1.2
>diff -u -r1.2 FilterIterator.java
>--- FilterIterator.java 2003/01/15 21:45:23 1.2
>+++ FilterIterator.java 2003/01/30 07:04:53
>@@ -69,10 +69,11 @@
>  * returned.
>  *
>  * @since Commons Collections 1.0
>- * @version $Revision: 1.1.1.1 $ $Date: 2003/01/29 13:40:37 $
>+ * @version $Revision: 1.2 $ $Date: 2003/01/15 21:45:23 $
>  *
>  * @author <a href="mailto:jstrachan@apache.org">James Strachan</a>
>  * @author Jan Sorensen
>+ * @author Ralph Wagner
>  */
> public class FilterIterator extends ProxyIterator {
>
>@@ -150,13 +151,20 @@
>     }
>
>     /**
>-     * Always throws UnsupportedOperationException as this class
>-     * does look-ahead with its internal iterator.
>-     *
>-     * @throws UnsupportedOperationException  always
>+     * Removes from the underlying collection of the base iterator the
>last
>+     * element returned by this iterator.
>+     * This method can only be called,
>+     * if <code>next()</code> was called, but not after
>+     * <code>hasNext()</code>, because the <code>hasNext()</code>
call
>+     * changes the base iterator.
>+     * @throws IllegalStateException if <code>hasNext()</code> has
already
>+     * been called.
>      */
>     public void remove() {
>-        throw new UnsupportedOperationException();
>+        if (nextObjectSet){
>+            throw new IllegalStateException();
>+        }
>+        getIterator().remove();
>     }
>
>
>Index: TestFilterIterator.java
>===================================================================
>RCS file: /home/cvspublic/jakarta-
>commons/collections/src/test/org/apache/commons/collections/iterators/T
estF
>ilterIterator.java,v
>retrieving revision 1.4
>diff -u -r1.4 TestFilterIterator.java
>--- TestFilterIterator.java 2002/12/13 12:03:06 1.4
>+++ TestFilterIterator.java 2003/01/30 08:00:33
>@@ -65,13 +65,17 @@
>
> import junit.framework.TestSuite;
> import junit.framework.Test;
>+import java.util.ArrayList;
>+import java.util.Arrays;
> import java.util.Iterator;
>+import java.util.List;
> import java.util.NoSuchElementException;
> import org.apache.commons.collections.Predicate;
>
> /**
>  *
>  * @author  Jan Sorensen
>+ * @author Ralph Wagner
>  */
> public class TestFilterIterator extends TestIterator {
>
>@@ -81,6 +85,7 @@
>     }
>
>     private String[] array;
>+    private List list;
>     private FilterIterator iterator;
>     /**
>      * Set up instance variables required by this test case.
>@@ -121,7 +126,9 @@
>      * @return
>      */
>     public Iterator makeFullIterator() {
>-        return makePassThroughFilter(new ArrayIterator(array));
>+        array = new String[] { "a", "b", "c" };
>+        list = new ArrayList(Arrays.asList(array));
>+        return makePassThroughFilter(list.iterator());
>     }
>
>     public Object makeObject() {
>@@ -129,7 +136,7 @@
>     }
>
>     public boolean supportsRemove() {
>-        return false;
>+        return true;
>     }
>
>
>@@ -184,10 +191,20 @@
>             assertTrue(i == elements.length - 1 ? !iterator.hasNext()
:
>iterator.hasNext());
>         }
>         verifyNoMoreElements();
>+
>+        // test removal
>+        initIterator();
>+        iterator.setPredicate(pred);
>+        if (iterator.hasNext()){
>+            Object last = iterator.next();
>+            iterator.remove();
>+            assertTrue("Base of FilterIterator still contains removed
>element.",
>+                !list.contains(last));
>+        }
>     }
>
>     private void initIterator() {
>-        iterator = makePassThroughFilter(new ArrayIterator(array));
>+        iterator = (FilterIterator) makeFullIterator();
>     }
>
>     /**
>
>_______________________________________________________________________
____
>___
>Schon wieder Viren-Alarm? Bei WEB.DE FreeMail ist das kein Problem,
>hier ist der Virencheck inklusive!
>http://freemail.web.de/features/?mc=021158
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message