I’ve come across some code that is using – and overusing – IEnumerable<T>, and thought it might be of general interest.

Consider the following method signatures:

IEnumerable<string> Process(List<string> input);

IList<string> Process(IEnumerable<string> input);

Of the two, which is the better choice? Write down your answer.

There are really two questions – what is the best choice for input parameters, and what is the best choice for return values. I’ll cover them separately:

Small In

The input parameter should be the smallest (ie least functional) type that allows the method to efficiently do what it needs to do. I’ve seen methods like this:

void Process(List<string> input)
{
    foreach (string in input)
    {
        …
    }
}

(or maybe the Linq equivalent).

In this case, you are just making things harder for the caller; all you really need is an IEnumerable<string>, so that’s what you should specify. On the other hand, I’ve also seen code like this:

void Process(IEnumerable<string> items, IEnumerable<ReferenceItem> referenceItems)
{
    var lookup = referenceItems.ToDictionary(referenceItem => referenceItem.Name, referenceItem => referenceItem.Contents)’
    …
}

This code takes a simple enumerable, and constructs a dictionary out of it. This is worse than the first case; if you need a dictionary, ask for a dictionary.

Big Out

Output parameters should be the biggest (ie most functional) type that is acceptable for the scenario. I’ve seen methods like this:

IEnumerable<string> Process(List<string> input)
{
    List<string> items = new List<string>();

    // fill list here

    return items;
}

The developer has created a beautiful, functional list object, but they’re only going to let the caller enumerate over it. This often leads to my favorite outcome, where the caller writes something like:

List<string> items = new List<string>(myClass,Process(input));

Don’t do that. Just return the List<string> or perhaps the IList<string>.

Return values aren’t as cut-and-dried as input parameters, however. If we modify the previous example as follows:

IList<string> Process(List<string> input)
{
    m_items = new List<string>();

    // fill list here

    return m_items;
} 

In this example, the class is retaining ownership of the list, and in that case, it probably doesn’t want to give it out to somebody who could clear it, or replace all the strings with the string “Haddock”. If the class is going to retain ownership, it should use a return type that prevents bad things like that from happening.