Reconsider implementation of Iterable

[robertvazan]

Implementing Iterable on StreamEx is inevitably incorrect, because there is no way for streams to satisfy the requirement of repeated iteration that comes with Iterable. It is an easy mistake to pass StreamEx to a method taking Iterable, which will then attempt iteration twice, because it relies on semantics of Iterable.

fn(StreamEx.of("one", "two"));

void fn(Iterable<String> items) {
    for (String item : items)
        g1(item);
    for (String item : items)
        g2(item);
}

Stream was designed to be independent of Iterable with no inheritance between the two. This means that library APIs will be providing convenience overloads for both Stream and Iterable, assuming no ambiguity between the two. When StreamEx is used with such overloads, it results in ambiguous method error from the compiler. You have run into this problem yourself in #146.

void fn(Iterable<String> items) {
}
void fn(Stream<String> stream) {
}

fn(StreamEx.of("one", "two"))

Given the two reasons above, it would be probably a good idea to remove Iterable implementation from StreamEx. Since the implementation of Iterable is unfixably broken anyway, I would argue it should be removed without consideration for backward compatibility. This would break builds of buggy code, but that might be seen as a feature. The only downside of this change is that the use of StreamEx in for loops will require explicit toList() or some other additional method call.

[amaembo]

The Iterable interface specification doesn't require multiple iterations. Moreover, there's an example in JDK itself that violates this: java.nio.file.DirectoryStream. The StreamEx javadoc contains a warning about this behavior detail. Using toList() is often not an option as you cannot benefit from short-circuiting and in particular, cannot iterate over the infinite stream. Also, depending on the stream size it may require significant memory overhead. Finally removing Iterable would be a serious breaking change. The library is quite popular already, so clients won't be happy to see the broken code.

[Maaartinus]

@amaembo IMHO the best solution would be not to implement Iterable and provide a method toOneTimeIterable(), which would clearly state what's going on. The operation would be sort-of no-op, copying nothing. It's still a breaking change (with a trivial and zero-overhead fix; unlike using toList()).