Hi Stuart,
The Alternatives section of the proposal is very thorough and it
mentions the following alternative:
"
An alternative adapter is to add a default method to Stream:
default Iterable<T> asIterable() { return this::iterator; }
for (T t : asIterable(stream)) {
...
}
This is slightly better at the call site, and it’s restricted to
streams. But it’s still not as nice as IterableOnce and it facilitates
creation of poorly-behaved Iterable instances.
"
It think this alternative is not given fair comparison. 1st this is an
instance method, so the foreach loop should read:
for (T t : stream.asIterable()) {
...
}
...which is better in particular when creation of Stream and consumption
is performed in single expression (not a good example, since it doesn't
close the stream):
for (String line : Files.lines(path).asIterable()) {
...
}
In addition, why should this be restricted to streams? This could be
combined with IterableOnce. The method could be called differently and
specified as BaseStream's terminal operation, returning IterableOnce<T>
instead of Iterable<T>:
/**
* Returns an {@link IterableOnce} for elements of this stream.
*
* <p>This is a <a href="package-summary.html#StreamOps">terminal
* operation</a>.
*
* @return iterable for elements of this stream that may be
iterated only once
*/
default IterableOnce<T> iterate() {
Iterator<T> iterator = iterator();
return () -> iterator; // this should be a proper IterableOnce
with exception thrown etc.
}
So IntStream and friends would get this too, although with necessary
boxing, which should be easy for JIT to eliminate and without conflicts
between IntStream.forEach and Iterable.forEach.
What do you think? Is this additional method invocation too much of a
drawback?
And now for something more controversial...
Is changing the language really out of the picture here?
The Iterator interface has got a new default method called
forEachRemaining in JDK 8. This method could be seen roughly equivalent
to one-shot iteration directly from IterableOnce:
IterableOnce.forEach() vs. IterableOnce.iterator().forEachReamining()
Both of above can only be performed once. And once is enough for foreach
loop. So if foreach loop was retrofitted to also act on Iterator(s) as a
possible right hand side (arrays, Iterable(s), Iterator(s)), then we
might not need this IterableOnce at all. You could then just write:
for (T t : stream.iterator()) ...
But that's probably not going to happen right?
Regards, Peter
On 3/12/19 10:59 PM, Stuart Marks wrote:
Hi Stephen,
My slight concern is that the terminal operation is hidden and not
immediately visible, which could be surprising. I do note that streams
throw a clear exception if a terminal operation is called a second
time which mitigates this to some degree.
Yes, this is certainly a possibility.
I'll note that even though my example does it, I think that storing a
stream in a local variable is a bit of a code smell. It has to be done
for try-with-resources, but storing the head of a pipeline in a local
variable so that it can be iterated over does introduce the
possibility of accidental reuse.
I suspect that most cases (aside from try-with-resources) will call a
method returning a fresh Stream that's then iterated over. For example,
for (var x : getSomeStream()) {
// ...
}
In this case there's no possibility of accidental reuse.
The IterableOnce class-level Javadoc contradicts the method-level
Javadoc. The class-level say "It is recommended that" whereas the
method-level Javadoc mandates.
Oh yes, thanks. I'll fix this to make the behavior mandatory.
s'marks
