Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concat applied after filter produces unexpected result #23

Closed
kotucz opened this issue Feb 2, 2016 · 3 comments
Closed

Concat applied after filter produces unexpected result #23

kotucz opened this issue Feb 2, 2016 · 3 comments

Comments

@kotucz
Copy link

kotucz commented Feb 2, 2016

Applying concat() on stream that is result of filter() produces incorrect result.
Below is new test that reproduces the issue:

@Test
public void testConcatOfFilter() {
    final PrintConsumer consumer = new PrintConsumer();
    final Stream<Integer> first = Stream.ofRange(0, 5)
        .filter(it -> true);
    final Stream<Integer> second = Stream.ofRange(5, 10)
        .filter(it -> true);
    Stream.concat(first, second)
        .forEach(consumer);
    assertEquals("0123456789", consumer.toString());
}

org.junit.ComparisonFailure: 
Expected :0123456789
Actual   :13null56789

The issue is that hasNext() implementation in filter() is not idempotent - when called multiple times, value is skipped.
In concat() implementation the (filter() hasNext()) is called once in hasNext() and once in next(), which would be ok, if filter() hasNext() did not have the side-effect.

aNNiMON added a commit that referenced this issue Feb 2, 2016
aNNiMON added a commit that referenced this issue Feb 2, 2016
@aNNiMON
Copy link
Owner

aNNiMON commented Feb 2, 2016

Fixed! Thank you so much, @kotucz

@aNNiMON aNNiMON closed this as completed Feb 2, 2016
@kotucz
Copy link
Author

kotucz commented Feb 3, 2016

Thanks for the quick fix!

Just one note - this implementation assumes that hasNext() is called at least once before next().
So just calling next() could break something (but this probably will never happens inside the library).
For inspiration the same thing is in guava Iterators.filter() (which is not a lightweight dependency though).

@aNNiMON
Copy link
Owner

aNNiMON commented Feb 3, 2016

Fixed with 80885bb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants