Hi Jonathan!
Funnily enough, I spotted that too on Thursday / Friday and I think its now
fixed it in CVS.
You can browse the code from CVS here:-
http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/dom4j/dom4j/src/java/org/dom4
j/tree/
http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/dom4j/dom4j/src/java/org/dom4
j/tree/FilterIterator.java?rev=1.3&content-type=text/vnd.viewcvs-markup
I think we both patched FilterIterator along the same lines (kinda spooky
that we both did it around the same time!).
I went the whole hog and implemented the NoSuchElementException throwing if
you call next() when there are no more elements - so I think we have a fully
supported FilterIterator now.
I also added your JUnit test case to CVS and added you as a contributor.
These changes should all show up in todays build and the current CVS image
appears to pass all JUnit tests with flying colours.
Many thanks for your work Jonathan!
James
----- Original Message -----
From: "Jonathan Doughty" <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>
Sent: Monday, August 06, 2001 3:52 AM
Subject: [dom4j-dev] Re: Incorrect implementation of hasNext in
FilterIterator
> 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
>
_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com
_______________________________________________
dom4j-dev mailing list
[EMAIL PROTECTED]
http://lists.sourceforge.net/lists/listinfo/dom4j-dev
