A problem with exception handling for IEnumerable<T>, it's lazyness-dependent.

Question

I used to create interfaces with IEnumerable<T> as return type, whenever I want to specify that a particular output is read-only. I like it as it's minimalistic, hides implementation details and decouples a callee from a caller.

But recently a colleague of mine argued that IEnumerable<T> should be kept for scenarios which involve lazy evaluation only, as otherwise it's unclear for a caller method, where the exception handling should take it's place -- around a method call or around an iteration. Then for the eager evaluation cases with a read-only output I should use a ReadOnlyCollection.

Sounds quite reasonable for me, but what would you recommend? Would you agree with that convention for IEnumerable? Or are there better methods for exception handling around IEnumerable?

Just in case if my question is unclear, I made a sample class illustrating the problem. Two methods in here have exactly the same signature, but they require different exception handling:

public class EvilEnumerable
{
    IEnumerable<int> Throw()
    {
        throw new ArgumentException();
    }

    IEnumerable<int> LazyThrow()
    {
        foreach (var item in Throw())
        {
            yield return item;
        }
    }

    public void Run()
    {
        try
        {
            Throw();
        }
        catch (ArgumentException)
        {
            Console.WriteLine("immediate throw");
        }

        try
        {
            LazyThrow();
        }
        catch (ArgumentException)
        {
            Console.WriteLine("No exception is thrown.");
        }

        try
        {
            foreach (var item in LazyThrow())
            {
                //do smth
            }
        }
        catch (ArgumentException)
        {
            Console.WriteLine("lazy throw");
        }
    }
}

Update 1. The question is not limited to ArgumentException only. It's about best practices for making friendly class interfaces, that tell you whether they return lazy evaluated result or not, because this influences the exception-handling approach.

Answer 1

The real problem here is the deferred execution. In the case of argument-checking, you can do this by adding a second method:

IEnumerable<int> LazyThrow() {
     // TODO: check args, throwing exception
     return LazyThrowImpl();
}
IEnumerable<int> LazyThrowImpl() {
    // TODO: lazy code using yield
}

Exceptions happen; even for non-deferred result (List<T>, for example) you can get errors (perhaps if another thread adjusts the list while you are iterating). The above approach lets you do as much as possible ahead of time to reduce the unexpected side-effects of yield and deferred execution.

Answer 2

I do not agree with your collegue. The documentation of the method shoul follow the standard an document which exceptions it might throw. Declaring the return type as IEnumerable does not make it read only. Whether or not it's read only dependts on the actual implementation of the interface being returned. The caller shouldnt rely on it being writable but that's very different from the collection being read only

Answer 3

Similar problem was discussed in Eric Lippert's posts:

http://blogs.msdn.com/ericlippert/archive/2007/09/05/psychic-debugging-part-one.aspx

http://blogs.msdn.com/ericlippert/archive/2007/09/06/psychic-debugging-part-two.aspx

However, I will not treat it as an argument against IEnumerable in general. To be honest, most times when collection I'm enumerating throws an exception I don't know what to do with it and basically it goes to some global error handler. Of course, I agree that the moment the exception is thrown may be confusing.

Answer 4

In theory, I agree with your colleague. If there is some 'work' to create the results, and that work may cause an exception, and you don't need lazy evaluation, then indeed making the result lazy will just complicate matters.

However in practice, there's no way you can look at the return type and infer anything too useful. Even if you made the return type ReadonlyCollection, it still might delay the evaluation and throw, for example (ReadonlyCollection is not sealed, so a subclass could explicitly implement the IEnumerable interface and do wacky things).

At the end of the day, the .Net type system is not going to help you reason much about the exception behavior of most objects.

I would, in fact, still choose to return a ReadonlyCollection rather than an IEnumerable, but because it exposes more useful methods (like O(1) access to the nth element) compared to IEnumerable. If you're going to produce a manifest collection backed by an array, you might as well give consumers back useful aspects of that. (You might also just return a 'fresh' array that the caller gets to own.)