views:

93

answers:

2

I've got a bunch of classes that can Process() objects, and return their own objects:

public override IEnumerable<T> Process(IEnumerable<T> incoming) { ... }

I want to write a processor class that can wrap one of these processors, and log any uncaught exceptions that the wrapped Process() method might throw. My first idea was something like this:

public override IEnumerable<T> Process(IEnumerable<T> incoming) {
    try {
        foreach (var x in this.processor.Process(incoming)) {
            yield return x;
        }
    } catch (Exception e) {
        WriteToLog(e);
        throw;
    }
}

but this doesn't work, due to CS1626: Cannot yield a value in the body of a try block with a catch clause.

So I want to write something that's conceptually equivalent but compiles. :-) I've got this:

public override IEnumerable<T> Process(IEnumerable<T> incoming) {
    IEnumerator<T> walker;
    try {
        walker = this.processor.Process(incoming).GetEnumerator();
    } catch (Exception e) {
        WriteToLog(e);
        throw;
    }

    while (true) {
        T value;
        try {
            if (!walker.MoveNext()) {
                break;
            }
            value = walker.Current;
        } catch (Exception e) {
            WriteToLog(e);
            throw;
        }
        yield return value;
    }
}

but that's more complex than I'd hoped, and I'm not entirely certain of either its correctness or that there isn't a much simpler way.

Am I on the right track here? Is there an easier way?

A: 

The loop and yield are not needed (at least in your example). Simple remove them and there is no longer a restriction on catch blocks.

public override IEnumerable<T> Process(IEnumerable<T> incoming) {
    try {
        return this.processor.Process(incoming);
    } catch (Exception e) {
        WriteToLog(e);
        throw;
    }
}

I am assuming this.processor.Process(incoming) returns a collection implementing IEnumerable<T>, so therefore you do not need to create a new iterator. If this.processor.Process(incoming) is lazy evaluating then

public override IEnumerable<T> Process(IEnumerable<T> incoming) {
    try {
        return this.processor.Process(incoming).ToList();
    } catch (Exception e) {
        WriteToLog(e);
        throw;
    }
}
Bear Monkey
@Bear Monkey, this will not work. Remember, enumerables are lazily evaluated. If there is an exception upon iterating through the enumerable that will happen **after** this method is called (when the enumerable itself is evaluated by the caller via `GetEnumerator()`) and thus the exception you're catching above will never actually get caught.
Kirk Woll
where does it say `this.processor.Process(incoming)` is lazy?
Bear Monkey
@Bear Monkey, I will rephrase. If the exception happens upon invocation of `GetEnumerator()` then your code will not catch it because it never invokes `GetEnumerator()`.
Kirk Woll
@Kirk you make no sense.
Bear Monkey
@Bear Monkey, what doesn't make sense?
Kirk Woll
@Kirk i know what your talking about but your assuming that this.processor.Process(incoming) is lazy just because its caller is returning IEnumerable<T>. Just because its returning IEnumerable<T> doesnt mean it has to be lazy at all.
Bear Monkey
@Bear Monkey, you misunderstand. The entire contract of `IEnumerable` is lazy -- that's what it means for it to have a `GetEnumerator()` method that returns an `IEnumerator`. Therefore all usages of `IEnumerable` are automatically lazy. Your update to your answer will technically work, but it somewhat ruins the pull-as-you-go contract of an IEnumerable. Since Matt's suggestion would not exhibit that limitation, I prefer his answer.
Kirk Woll
@Kirk No that is not true. By your definition a List<T> is always lazy evaluated because it implements IEnumerable<T> which it clearly is not. He wanted an easier solution so i gave him one.
Bear Monkey
@Bear Monkey, even a List<T> is technically evaluated lazily. Not until you invoke `GetEnumerator()` has anything happened (such as instantiating the enumerator) -- before then you are simply passing around references.
Kirk Woll
@Kirk err no... List<T> is not evaluated lazily. List<T> evaluation is not performed by its GetEnumerator() all that does is return an IEnumerator<T> to iterate over the collection.
Bear Monkey
@Bear Monkey, I wonder what it is you think "lazily" means...
Kirk Woll
@kirk The IEnumerator<T> returned by List<T>.GetEnumerator() is not lazy evaluating the list. All its does is to iterate over the collection. Other collections may lazy evaluate when you iterate over them but List<T> isn't one of them.
Bear Monkey
A: 

It might be nicer if the process object was something that processed just one item in the list at a time (if that's even possible), that way you could collect each individual exception and return an AggregateException just like the Task Parallel library does.

[If process operates on the list as a whole, or if a single exception needs to abort the whole process obviously this suggestion isn't appropriate.]

Hightechrider
It might be nicer, or it might not, but that's a (semi-external) interface I can't change, at least not on a reasonable timescale. :-)
Ken