[Collections][PATCH] Enable remove in FilterIterator

2003-01-30 Thread Ralph Wagner
 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

2003-01-30 Thread Shapira, Yoav
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

2003-01-30 Thread Stephen Colebourne
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