DATACMNS-650 - Provide a CloseableIterator abstraction as a foundation for streaming of results.

[thomasdarimont]

Introduced CloseableIterator, AutoCloseableIterator and ForwardingAutoCloseableIterator to bridge between the two.

[jhoeller]

Guys, java.io.Closeable extends java.lang.AutoCloseable anyway... So I'm not sure what AutoCloseableIterator really adds here, and why the forwarding is needed?

We're implementing Java 6's Closeable to implicitly get Java 7 AutoCloseable behavior in a few places in the core framework as well. Seems to work fine for those purposes.

[thomasdarimont]

AutoCloseable was just introduced with Java 7 so it isn't available if SD Commons is used with Java 6.
Or am I missing something here?

[jhoeller]

Sure, but my point is: Any of our classes implementing Closeable implicitly implements AutoCloseable on Java 7, since the Closeable interface itself happens to extend AutoCloseable on Java 7+. This is perfectly Java 6 compatible and also enables AutoCloseable when running on Java 7, simply through implementing Closeable. Any extra business with AutoCloseable decoration is effectively unnecessary.

[thomasdarimont]

A gotcha :) You are right.

[odrotbohm]

Can't we just use ClassUtils.forName(…) here?

[odrotbohm]

As outline in the comments on the original pull request I think we should focus on a CloseableIterable as return type. Using an Iterator as return type seems rather uncommon.

[thomasdarimont]

After having played a bit with the API I still think that a CloseableIterable<T>provides a richer API experience.

Consider the following variants:

//Variant 1 - CloseableIterator<Person>
@Query("{firstname:{$in:?0}}")
CloseableIterator<Person> findByCustomQueryByFirstnames(List<String> firstnames);

//V1: Use case 1 - loop over all results.
try (CloseableIterator<Person> result = repository.findByCustomQueryByFirstnames(Arrays.asList("Dave"))){
    for (Person person : result) {
        readPersons.add(person);
    }
}

//V1: Use case 2 - check for results and process later
try (CloseableIterator<Person> result = repository.findByCustomQueryByFirstnames(Arrays.asList("Dave"))){
    if(result.hasNext()){
        //doSomething
    }else{
        //doSomething else
    }

    for (Person person : result) {
        readPersons.add(person);
    }   
}


//Variant 2 - CloseableIterable<Person>
@Query("{firstname:{$in:?0}}")
CloseableIterable<Person> findByCustomQueryByFirstnames(List<String> firstnames);

//V2: Use case 1 - loop over all results.
try (CloseableIterable<Person> result = repository.findByCustomQueryByFirstnames(Arrays.asList("Dave"))){
    for (Person person : result) {
        readPersons.add(person);
    }
}

//V2: Use case 2 - check for results and process later
try (CloseableIterable<Person> result = repository.findByCustomQueryByFirstnames(Arrays.asList("Dave"))){

    Iterator iter = result.iterator();

    if(iter.hasNext()){
        //doSomething
    } else{
        //doSomething else
    }

    for (Person person : result) {
        readPersons.add(person);
    }   

    //what to close now? iterator or result?
}

In Variant1 it is clear what the "resource" and a user can conveniently iterate over the results.
Variant2 however doesn't make this clear per se - IMHO.

Variant1 is also much easier to work with (older) Collection libraries that don't support Iterable yet - so one can safe a method call here... it is not that important but adds to the list IMHO.