Hello!

The proposal looks good to me. In particular it's very nice to have
this properly (ability to iterate only once) to be encoded in the type
system. This could be helpful for static analysis tools to warn when
IterableOnce is reused.

Scanner case looks funny. In general such pattern could be applied to
every Iterator implementor.

As for Streams, I worry a little that people would prefer for(T t :
stream) {...} over stream.forEach(t -> ...) even when forEach works
perfectly. The Stream.iterator() implementation produces some amount
of unnecessary garbage. At least it would be nice to add special case
into Spliterators#iterator(java.util.Spliterator<? extends T>) to
reduce number of wrappers if stream was directly created from an
iterator or collection:

--- src/java.base/share/classes/java/util/Spliterators.java (revision
53626:e2fc434b410a35b28ab433c29863c8a26e4e813a)
+++ src/java.base/share/classes/java/util/Spliterators.java (date 1551417931182)
@@ -665,6 +665,24 @@
      */
     public static<T> Iterator<T> iterator(Spliterator<? extends T>
spliterator) {
         Objects.requireNonNull(spliterator);
+        if (spliterator instanceof IteratorSpliterator) {
+            @SuppressWarnings("unchecked")
+            IteratorSpliterator<? extends T> wrapper =
(IteratorSpliterator<? extends T>) spliterator;
+            wrapper.estimateSize(); // force initialization
+            Iterator<? extends T> it = Objects.requireNonNull(wrapper.it);
+            // Avoid exposing the remove() method of original
iterator; too bad there's no way to avoid this if remove() is not
implemented :(
+            return new Iterator<>() {
+                @Override
+                public boolean hasNext() {
+                    return it.hasNext();
+                }
+
+                @Override
+                public T next() {
+                    return it.next();
+                }
+            };
+        }
         class Adapter implements Iterator<T>, Consumer<T> {
             boolean valueReady = false;
             T nextElement;

Of course this could be done later as separate enhancement.

On Fri, Mar 1, 2019 at 9:47 AM Stuart Marks <stuart.ma...@oracle.com> wrote:
>
> Hi all,
>
> Please review and comment on this proposal to allow Stream instances to be 
> used
> in enhanced-for ("for-each") loops.
>
> Abstract
>
> Occasionally it's useful to iterate a Stream using a conventional loop. 
> However,
> the Stream interface doesn't implement Iterable, and therefore streams cannot 
> be
> used with the enhanced-for statement. This is a proposal to remedy that
> situation by introducing a new interface IterableOnce that is a subtype of
> Iterable, and then retrofitting the Stream interface to implement it. Other 
> JDK
> classes will also be retrofitted to implement IterableOnce.
>
> Full Proposal:
>
>      http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>
> Bug report:
>
>      https://bugs.openjdk.java.net/browse/JDK-8148917
>
> Webrev:
>
>      http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>
> Note, this changeset isn't ready to push yet. In particular, it has no tests
> yet. However, the implementation is so simple that I figured I should include
> it. Comments on the specification wording are also welcome.
>
> Thanks,
>
> s'marks

Reply via email to