Thinking about it a bit more, I realized that my patch only half fixed
the bug: even in my previous patch there is still the presumption that
one _must_ call hasNext() to advance to the next element in the
iteration. That shouldn't be the case: next() itself should so what's
necessary, there should not be a requirement to call hasNext().q
Below is a second patch, against the original, that hopefully addresses
all the requirements of being a true Iterator. Well, almost. It
appears that true Iterators throw a NoSuchElementException when one
attempts to do an extra next; FilterIterator as presently implemented
just sets the returned object to null. I decided not to add that more
correct behavior for fear of breaking code that doesn't expect it.
Perhaps that should be added to, however, since ...
Unfortunately my patch seems to cause a lot of failures in other test
cases in the dom4j test suite. Perhaps there is more dependence on the
original, non-standard Iterator behavior than I realized; and fixing
those is a bit beyond my capability/familiarity with dom4j.
Here is my new diff:
diff -u FilterIterator.java.orig FilterIterator.java
--- FilterIterator.java.orig Mon Jul 30 14:30:30 2001
+++ FilterIterator.java Sun Aug 5 22:25:43 2001
@@ -26,24 +26,32 @@
this.proxy = proxy;
}
-
- public boolean hasNext() {
+ private void advanceToNext() {
if ( proxy == null ) {
- return false;
+ return;
}
while (proxy.hasNext()) {
next = proxy.next();
- if ( matches(next) ) {
- return true;
+ if ( next == null || matches(next) ) {
+ return;
}
}
proxy = null;
- next = null;
- return false;
+ }
+
+ public boolean hasNext() {
+ if (next == null)
+ advanceToNext();
+
+ return next != null;
}
public Object next() {
- return next;
+ if (next == null)
+ advanceToNext();
+ Object toReturn = next;
+ next = null;
+ return toReturn;
}
public void remove() {
And here, for what it is worth, is a TestIterator set of test cases that
passes the way I expect for my version and fails the way I expect for
the original. You are welcome to add it to dom4j test set if you
choose. Thanks again for dom4j.
/*
* $Id$
*/
package org.dom4j;
import java.util.Iterator;
import java.util.List;
import junit.framework.*;
import junit.textui.TestRunner;
/** A test harness to test the Iterator API in DOM4J
*
* @author <a href="mailto:jdoughty@[EMAIL PROTECTED]";>Jonathan
Doughty</a>
* @version $Revision: 1.01 $
*/
public class TestIterator extends AbstractTestCase {
public static void main( String[] args ) {
TestRunner.run( suite() );
}
public static Test suite() {
return new TestSuite( TestIterator.class );
}
public TestIterator(String name) {
super(name);
}
protected Document iterDocument;
private static final int NUMELE = 10;
protected void setUp() throws Exception {
iterDocument = createDocument();
Element root = iterDocument.addElement( "root" );
for (int i = 0; i < NUMELE; i++) {
root.addElement( "iterator test")
.addAttribute( "instance", Integer.toString(i));
}
}
// Test case(s)
//-------------------------------------------------------------------------
public void testElementCount() throws Exception {
Element root = iterDocument.getRootElement();
assertTrue( "Has root element", root != null );
List elements = root.elements( "iterator test" );
int elementSize = elements.size();
assertTrue( "Root has " + elementSize + " children",
elements != null && elementSize == NUMELE );
}
public void testPlainIteration() throws Exception {
Element root = iterDocument.getRootElement();
List elements = root.elements( "iterator test" );
Iterator iter = root.elementIterator( "iterator test" );
int elementSize = elements.size();
int count = 0;
for (; iter.hasNext();) {
Element e = (Element) iter.next();
assertTrue("instance " + e.attribute("instance").getValue() +
" equals "+ count,
e.attribute("instance").getValue().equals(
Integer.toString(count)));
count++;
}
assertTrue( elementSize + " elements iterated", count ==
elementSize );
}
public void testSkipAlternates() throws Exception {
Element root = iterDocument.getRootElement();
List elements = root.elements( "iterator test" );
Iterator iter = root.elementIterator( "iterator test" );
int elementSize = elements.size();
int count = 0;
for (; iter.hasNext();) {
Element e = (Element) iter.next();
assertTrue("instance " + e.attribute("instance").getValue() +
" equals "+ count*2,
e.attribute("instance").getValue().equals(
Integer.toString(count*2)));
iter.next();
count++;
}
assertTrue( (elementSize/2) + " alternate elements iterated",
count == (elementSize/2) );
}
public void testNoHasNext() throws Exception {
Element root = iterDocument.getRootElement();
List elements = root.elements( "iterator test" );
Iterator iter = root.elementIterator( "iterator test" );
int elementSize = elements.size();
int count = 0;
Element e = null;
for (; count < elementSize; ) {
e = (Element) iter.next();
assertTrue("instance " + e.attribute("instance").getValue() +
" equals "+ count,
e.attribute("instance").getValue().equals(
Integer.toString(count)));
System.out.println("instance " +
e.attribute("instance").getValue() +
" equals "+ count);
count++;
}
try {
e = (Element) iter.next();
if (e != null) {
// Real Iterators wouldn't get here
assertTrue( "no more elements,value instead is " +
e.attribute("instance").getValue(), e == null);
}
}
catch (Exception exp) {
assertTrue( "Real iterators throw NoSuchElementException",
exp instanceof java.util.NoSuchElementException);
}
}
public void testExtraHasNexts() throws Exception {
Element root = iterDocument.getRootElement();
List elements = root.elements( "iterator test" );
Iterator iter = root.elementIterator( "iterator test" );
int elementSize = elements.size();
int count = 0;
for (; iter.hasNext();) {
Element e = (Element) iter.next();
assertTrue("instance " + e.attribute("instance").getValue() +
" equals "+ count,
e.attribute("instance").getValue().equals(
Integer.toString(count)));
iter.hasNext();
count++;
}
assertTrue( elementSize + " elements iterated with extra hasNexts",
count == elementSize );
}
}
_______________________________________________
dom4j-dev mailing list
[EMAIL PROTECTED]
http://lists.sourceforge.net/lists/listinfo/dom4j-dev
