RFR: 8072727 - add variation of Stream.iterate() that's finite

[Tagir Valeev]

Thank you for valueable coments, here's updated webrev:

http://cr.openjdk.java.net/~tvaleev/webrev/8072727/r2/

Changes:
- Spec updated according to Brian Goetz suggestion
- Predicate<? super T> wildcard
- two-arg versions of iterate() are rewritten with Spliterator and started
flag
- IterateTest rewritten: now
withData().stream().expectedResult().exercize() used; expected results
added.

With best regards,
Tagir Valeev

On Tue, Feb 16, 2016 at 7:32 PM, Paul Sandoz <paul.sandoz at oracle.com> wrote:

> Hi Tagir,
>
> The implementation approach looks good. I agree with Brian on the
> documentation, esp. about the connection to a for loop.
>
> Testing-wise adding to the spliterator data sources is ok. Because you
> have done that there is no need to explicitly use SpliteratorTestHelper in
> IterateTest, as that is reasonably covered by SpliteratorTest. Since you
> have the data providers for streams just run it through exerciseStream
> instead.
>
> Regarding wild cards we could clean this up later and include other
> methods [1]. It’s easy to go overboard on this, recommend sticking to ?
> extends/? super where appropriate.  The existing iterate methods use
> UnaryOperator, thus there is no wiggle room there, am i ok with reusing
> that for the 3-arg method.
>
> Regarding the existing two-args method, my inclination is to keep the
> implementations separate from the 3-args, but feel free to change to using
> abstract spliterator (reduce layering) using a similar code pattern (no
> need for "finished" or overriding forEachRemaining). Then you can remove
> Streams.NONE and it’s sneaky erased usage :-)
>
> Paul.
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8132097
>
> > On 14 Feb 2016, at 15:53, Tagir F. Valeev <amaembo at gmail.com> wrote:
> >
> > Hello!
> >
> > I wanted to work on foldLeft, but Brian asked me to take this issue
> > instead. So here's webrev:
> > http://cr.openjdk.java.net/~tvaleev/webrev/8072727/r1/
> >
> > I don't like iterator-based Stream source implementations, so I made
> > them AbstractSpliterator-based. I also implemented manually
> > forEachRemaining as, I believe, this improves the performance in
> > non-short-circuiting cases.
> >
> > I also decided to keep two flags (started and finished) to track the
> > state. Currently existing implementation of infinite iterate() does
> > not use started flag, but instead reads one element ahead for
> > primitive streams. This seems wrong to me and may even lead to
> > unexpected exceptions (*). I could get rid of "started" flag for
> > Stream.iterate() using Streams.NONE, but this would make object
> > implementation different from primitive implementations. It would also
> > be possible to keep single three-state variable (byte or int,
> > NOT_STARTED, STARTED, FINISHED), but I doubt that this would improve
> > the performance or footprint. Having two flags looks more readable to
> > me.
> >
> > Currently existing two-arg iterate methods can now be expressed as a
> > partial case of the new method:
> >
> > public static<T> Stream<T> iterate(final T seed, final UnaryOperator<T>
> f) {
> >    return iterate(seed, x -> true, f);
> > }
> > (same for primitive streams). I may do this if you think it's
> > reasonable.
> >
> > I created new test class and added new iterate sources to existing
> > data providers.
> >
> > Please review and sponsor!
> >
> > With best regards,
> > Tagir Valeev.
> >
> > (*) Consider the following code:
> >
> > int[] data = {1,2,3,4,-1};
> > IntStream.iterate(0, x -> data[x])
> >         .takeWhile(x -> x >= 0)
> >         .forEach(System.out::println);
> >
> > Currently this unexpectedly throws an AIOOBE, because
> > IntStream.iterate unnecessarily tries to read one element ahead.
> >
>
>