Exception handling in Iterable

Question

Is there any way of handling -- and continuing from -- an exception in an iterator while maintaining the foreach syntactic sugar?

I've got a parser that iterates over lines in a file, handing back a class-per-line. Occasionally lines will be syntactically bogus, but that doesn't necessarily mean that we shouldn't keep reading the file.

My parser implements Iterable, but dealing with the potential exceptions means writing

for (Iterator iter = myParser.iterator(); iter.hasNext(); ) {
  try {
    MyClass myClass = iter.next();
    // .. do stuff ..
  } catch (Exception e) {
    // .. do exception stuff ..
  }
}

.. nothing wrong with that, but is there any way of getting exception handling on the implicit individual iter.next() calls in the foreach construct?

Answer 1

you could of course wrap your iterator in another iterator, something like this:

public class SafeIterator<E> implements Iterator<E>{
    private Iterator<E> inner;
    public SafeIterator(Iterator<E> inner){this.inner=inner;}
    public boolean hasNext(){return inner.hasNext();}
    public E next(){
        try{
            return inner.next();
        }catch(Exception e){return null;} //you'll also want to do some logging here
    }
} 

---

EDIT:

the trouble here is that a) you are breaking the iterator contract. hasNext() suggests that a value is coming next while this version might or might not provide a value (it might be null), so the client always has to do null checks:

for(MyType m : myIterator){
    if(m!=null){
        // do stuff here
    }
}

which of course takes most of the elegance away from the iterator pattern, but more importantly b) you are dropping exceptions. We are violating Josh Bloch (Effective Java) item #65: "Don't ignore exceptions". We're solving the wrong problem by ignoring bad values. The parser should either learn to deal with bad values (perhaps by skipping ahead to the next good value), or there shouldn't be any bad values.

Answer 2

Personally, I'd do this using guava's (formerly google-collections) AbstractIterator -- see the Javadoc for that class for a simple null-skipping example, which could be easily modified:

public static Iterator<MyClass> skipExceptions(final Iterator<MyClass> in) {
  return new AbstractIterator<MyClass>() {
    protected String computeNext() {
      while (in.hasNext()) {
        try {
           return in.next();
        } catch (MalformedThingyException e) {
           // Do nothing, skip to the next one
        }
      }
      return endOfData();
    }
  };
}}

then when using this it's as simple as:

for (MyClass thing : skipExceptions(myParser.iterator()) { 
   // Do something ONLY to those items that didn't cause an exception
}

Yes, it's slightly more verbose but it's also general-purpose and easily reusable (and very clean at the calling side).

Answer 3

I was pretty confused. I saw a simple iterator pattern, typical iter.hasNext() and iter.next() calls and asked myself, why one would need a special exception handling on those iterator operations. The iterator either has a next element and returns it from the collection or it doesn't and this is detected by the hasNext() call.

So I started to question everything and then it came to my mind, that your basically using the iterator pattern in a .. say .. unconventional way. Your not iterating through a collection or through lines of a file but your using Iterator#next() to parse a textbased model into objects.

Here's a short definition from wikipedia:

In object-oriented programming, the Iterator pattern is a design pattern in which iterators are used to access the elements of an aggregate object sequentially without exposing its underlying representation.

To me it's acceptable to use a textfile as a collection of lines and iterate. But then the iterator should return text lines (Strings). You really should separate iterating and parsing in your code, like illustrated in the following snippets. Either you have a parser that parses the text file and provides an iterator for the MyClass objects or you iterate over the text file and parse the lines one-by-one:

public void snippet1(String[] lines, MyParser, myParser) {
  for (String line:lines) {
    try {
      MyClass myClass = myParser.parse(line);
    } catch (Exception e) {
      // handle/report unparsable lines
    }
  }
}

public void snippet2(String[] lines) {
    try {
      MyParser myParser = new MyParser();
      myParser.parse(lines);
    } catch (Exception e) {
      // handle/report unparsable lines
    }
    for (MyClass myClass:myParser) {
      // do something with myClass object
    }
}

Answer 4

Is there any way of handling -- and continuing from -- an exception in an iterator while maintaining the foreach syntactic sugar?

There is no such sugar.

Occasionally lines will be syntactically bogus, but that doesn't necessarily mean that we shouldn't keep reading the file.

Well, if it's not that exceptional the lines are bogus, why throw exceptions? You could rework your iterator a bit. Assuming you currently iterate over ParsedThingy instances, and that the parser throws ThingyParseException if parsing fails, iterate over wrappers which let you query the parse results for bogusness, like this:

for (Possibly<ParsedThingy, ThingyParseException> p : parser) {
    if (p.exception() != null) handleException(p.exception());
    else doSomethingExcitingWith(p.value());
}

Somewhat more self-documenting than seemingly spontaneously returning nulls; it also lets you give information about the error to the client code.

Possibly<V, X> is a wrapper around a value which may in fact be an exception. You can query for an exceptional status by checking if exception() is non-null, and get the value for the non-exceptional case by calling value() (which will throw if it is an exception):

class Possibly<V, X extends Throwable> {
    private final V value;
    private final X exception;
    public static <V, X extends Throwable> Possibly<V, X> forValue(V v){ 
        return new Possibly<V, X>(v, null); 
    }
    public static <V, X extends Throwable> Possibly<V, X> forException(X x){ 
        if (x == null) throw new NullPointerException(); 
        return new Possibly<V, X>(null, x);
    }
    private Possibly(V v, X x){ value = v; exception = x; }
    public X exception(){ return exception; }
    public V value() throws X {
        if (exception != null) throw exception;
        return value;
    }
}

Then your iterator() will look something like this:

Iterator<Possibly<ParsedThingy, ThingyParseException>> parse() {
    return new Iterator<Possibly<ParsedThingy, ThingyParseException>> {
        public boolean hasNext(){ ... }
        public void remove(){ ... }
        public Possibly<ParsedThingy, ThingyParseException> next() 
            try {
                ParsedThingy t = parseNext(); // throws ThingyParseException
                return Possibly.forValue(t);
            } catch (ThingyParseException e) {
                return Possibly.forException(e);
            }
        }
    };
}

Kind of verbose, could be avoided by making stuff less generic.