[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/FilterIterator.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:[EMAIL PROTECTED];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 codenext()/code was called, but not after + * codehasNext()/code, because the codehasNext()/code call + * changes the base iterator. + * @throws IllegalStateException if codehasNext()/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/TestFilterIterator.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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
RE: [Collections][PATCH] Enable remove in FilterIterator
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:[EMAIL PROTECTED]] Sent: Thursday, January 30, 2003 3:28 AM To: [EMAIL PROTECTED] 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.java2003/01/15 21:45:23 1.2 +++ FilterIterator.java2003/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:[EMAIL PROTECTED];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 codenext()/code was called, but not after + * codehasNext()/code, because the codehasNext()/code call + * changes the base iterator. + * @throws IllegalStateException if codehasNext()/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.java2002/12/13 12:03:06 1.4 +++ TestFilterIterator.java2003/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: [EMAIL PROTECTED] For additional commands, e-mail
Re: [Collections][PATCH] Enable remove in FilterIterator
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 [EMAIL PROTECTED] 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:[EMAIL PROTECTED]] Sent: Thursday, January 30, 2003 3:28 AM To: [EMAIL PROTECTED] 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:[EMAIL PROTECTED];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 codenext()/code was called, but not after + * codehasNext()/code, because the codehasNext()/code call + * changes the base iterator. + * @throws IllegalStateException if codehasNext()/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