neo4j/community — Issue #141: Only implement Iterable where appropriate
Author: ractive
Created: 2011-12-20 16:04:55+00:00
State: open

This is a followup to [this mailing list topic](https://groups.google.com/forum/#!topic/neo4j/IefEwe2j6wY).

For the second time we stumbled about Iterables that are no Iterables in Neo4j. `IndexHits` e.g. extends `Iterator` and `Iterable` and states in its documentation that it is not really an iterable but always returns "this" in calls to iterator(). So far so good. At least the docu states that.

But now when using Spring Data Neo4j (and probably other frameworks), the `IndexHits` is wrapped and hidden behind other constructs like `ClosableIterable` and `IterableAdapter` etc. The user has no idea anymore that those IterableWrapper and ClosableIterable things he gets back are no iterables at all - well the names tell him of course that those things *are* Iterables. This leads to very, very strange behavior like this:

```java
@Test
public void ouch() {
Device d1 = new Device("d1").persist();
Device d2 = new Device("d2").persist();

ClosableIterable devices = deviceRepository().findAll();
assertThat(devices, hasItems(d1));
assertEquals(1, IteratorUtil.count(devices));
}
```

We add two devices to our DB, but the test clearly states that findAll() only returns one `(assertEquals(1, IteratorUtil.count(devices))`).

As an exercise for the reader: explain why this test misleads you....

An Iterable should always return a valid (new) iterator for every call to iterator(). Implementing Iterable just to be able to use an iterator in a for loop is evil..... >:-)

I'm not sure if there are other classes misleadingly implementing Iterable, but IndexHits definitively must not implement it.