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

[Tagir Valeev]

Hello!

Here's updated webrev:
http://cr.openjdk.java.net/~tvaleev/webrev/8072727/r3/
All mentioned issues were fixed. I agree that using new features like
List.of is better, because this way it's easier to find problems with them
prior to release.

With best regards,
Tagir Valeev.

On Mon, Feb 22, 2016 at 2:50 PM, Paul Sandoz <paul.sandoz at oracle.com> wrote:

> Hi Tagir,
>
> > On 21 Feb 2016, at 13:15, Tagir Valeev <amaembo at gmail.com> wrote:
> >
> > 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.
> >
>
> Looking good. Just minor comments.
>
> For the primitive streams
> —
>
> +        Spliterator.OfDouble spliterator = new
> Spliterators.AbstractDoubleSpliterator(Long.MAX_VALUE,
> +               Spliterator.ORDERED | Spliterator.IMMUTABLE) {
> +            double prev;
> ...
> -        return
> StreamSupport.doubleStream(Spliterators.spliteratorUnknownSize(
> -                iterator,
> -                Spliterator.ORDERED | Spliterator.IMMUTABLE |
> Spliterator.NONNULL), false);
>
> We should retain the NONNULL characteristic.
>
> Thinking a little more that is something that should perhaps be added by
> default by the primitive abstract spliterators. Something to consider later
> maybe.
>
>
> Stream
> —
>
>      public static<T> Stream<T> iterate(final T seed, final
> UnaryOperator<T> f) {
>          Objects.requireNonNull(f);
>
> -        final Iterator<T> iterator = new Iterator<T>() {
> -            @SuppressWarnings("unchecked")
> -            T t = (T) Streams.NONE;
>
> I believe you can now remove the field Streams.NONE?
>
>
> 1186     public static<T> Stream<T> iterate(final T seed, final
> UnaryOperator<T> f) {
> 1187         Objects.requireNonNull(f);
> 1188         Spliterator<T> spliterator = new
> Spliterators.AbstractSpliterator<T>(Long.MAX_VALUE,
> 1189                Spliterator.ORDERED | Spliterator.IMMUTABLE) {
>
> I forgot to mention before you can now use the “<>" on the RHS, although
> IDEs might not like it javac will :-)
>
>
> 1270             @Override
> 1271             public void forEachRemaining(Consumer<? super T> action) {
> 1272                 Objects.requireNonNull(action);
> 1273                 if (finished)
> 1274                     return;
> 1275                 T t = started ? f.apply(prev) : seed;
> 1276                 finished = true;
>
> A really minor thing but if you move “finished = true” above one line, it
> means there is one less place where unintended recursive traversal can
> occur.
>
>
> IterateTest
> —
>
>   57             {Arrays.asList(1, 2, 4, 8, 16, 32, 64, 128, 256, 512),
>   58                 Factory.ofSupplier("ref.ten", () -> Stream.iterate(1,
> x -> x < 1000, x -> x * 2))},
>
> We now have the method “List.of", up to you if you wanna update to use
> that or not.
>
>
> Thanks,
> Paul.
>
>
>