FilteringIterator.hasNext advances iterator every time it is called.

[GoogleCodeExporter]

I have attached a patch with the fix to FilteringIterator and an associated 
unit test.

What steps will reproduce the problem?

1. Return a ResultSet that will apply filtering.
2. Use the returned Iterator's hasNext() function several times.
3. Note that the value of next() is no longer the first value of the ResultSet.

What is the expected output? What do you see instead?

Calling hasNext() on an iterator should always return true if a call to next 
will return an object.  It should not advance the pointer as this will cause 
skipping of values.

What version of the product are you using? On what operating system?
1.2.2

Please provide any additional information below.

This was exposed by attempting to copy a ResultSet into a Guava immutable 
collection. They use hasNext() to verify the size of the collection and this 
was causing the iterator to skip values.  

Original issue reported on code.google.com by gabe.hi...@gmail.com on 26 Sep 2013 at 5:25

Attachments:

• filtering_iterator.diff

[GoogleCodeExporter]

Yes this is an issue, thanks for reporting it.

hasNext() does advance the pointer. It does not affect typical usage of the 
iterator, but it's wrong nonetheless.

But I have not been able to reproduce that particular issue with Guava. The 
following copies from a FilteringIterator into a Guava ImmutableCollection and 
the test passes:

    @Test
    public void testFilteringIterator() {
        List<String> testList = Arrays.asList("abc", "bcd", "cde");
        FilteringIterator<String> iterator = new FilteringIterator<String>(testList.iterator()) {
            @Override
            public boolean isValid(String object) {
                return true;
            }
        };
        ImmutableCollection<String> immutableCollection = ImmutableList.<String>builder().addAll(iterator).build();
        assertThat(immutableCollection.size(), is(3));
        assertThat(immutableCollection.toString(), is("[abc, bcd, cde]"));
    }

It's possible there is some other way of copying into an ImmutableCollection 
that I'm not aware of could hit this issue though. As a workaround in the 
meantime the usage above seems to work for me.

Thanks for reporting this, it will be fixed!

Original comment by ni...@npgall.com on 27 Sep 2013 at 2:16

• Changed state: Accepted

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Sorry cut and past error.

It occurs when using ImmutableSet/List.copyOf(Iterator) as it uses the 
hasNext() to optimize the returned List / Set.  This reproduction case works 
with Guava 13 and on.  I had never attempted to use the builder in place of the 
copyOf method and so hit this right away.  

    @Test
    public void testFilteringIterator() {
        List<String> testList = Arrays.asList("abc", "bcd", "cde");
        FilteringIterator<String> iterator = new FilteringIterator<String>(testList.iterator()) {
            @Override
            public boolean isValid(String object) {
                return true;
            }
        };
        ImmutableCollection<String> immutableCollection = ImmutableList.copyOf(iterator);
        assertThat(immutableCollection.size(), is(3));
        assertThat(immutableCollection.toString(), is("[abc, bcd, cde]"));
    }

Original comment by gabe.hi...@gmail.com on 27 Sep 2013 at 5:20

[GoogleCodeExporter]

Original comment by ni...@npgall.com on 2 Oct 2013 at 9:36

• Changed state: Started

[GoogleCodeExporter]

Fixed in trunk, by applying your patch. Many thanks Gabe!

I will leave this issue open until included in the next binary release, 
hopefully next week or so.

Original comment by ni...@npgall.com on 2 Oct 2013 at 10:45

[GoogleCodeExporter]

Fixed in release 1.2.4. Many thanks!

Original comment by ni...@npgall.com on 22 Nov 2013 at 10:01

• Changed state: Fixed