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 <amae...@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. >
