Udostępnij za pośrednictwem

Small in, Big out

  • Artykuł

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.

Comments

  • Anonymous
    November 19, 2012
    That example: IEnumerable<string> Process(List<string> input)  {    List<string> items = new List<string>();    // fill list here    return items; } Should often be replaced with code that never even builds the list in the first place, i.e. using yield return. That's the best reason to use IEnumerable<T> everywhere (in input and output parameters) I've found yet.

  • Anonymous
    November 19, 2012
    It's not the return type that's important in your last example but what's being returned. Changing the return type to IEnumerable&lt;string&gt; won't make it anymore safe but changing the return statement to m_items.ToList() without changing the return type would

  • Anonymous
    November 19, 2012
    Thank you for this helpful advice.

  • Anonymous
    November 19, 2012
    Thanks for sharing this!  It is one item I've been pointing out in our internal code reviews in recent months.  We definitely want to make our method signatures as useful and friendly to the calling applications as we can.

  • Anonymous
    November 20, 2012
    The comment has been removed

  • Anonymous
    November 20, 2012
    Some people will argue that IList has Add, Remove, Insert methods - and I agree in sorts there's space for an immutable list interface which only includes .Count and an indexer, however this is not a good reason to fall back to IEnumerable for the previous reasons. We have ReadOnlyCollection and IList.IsReadOnly for immutable collections for now.

  • Anonymous
    November 20, 2012
    The comment has been removed

  • Anonymous
    November 20, 2012
    .... ah yeah, @configurators post reminds me - one last thing on this. I return IEnumerable<T> a fair bit too, where appropriate - the yield pattern is a great example of this

  • Anonymous
    November 27, 2012
    I've never encountered a for List<> with 2 parameters as you have in your example: new List<string>(myClass,Process(input)); What does that do?

  • Anonymous
    November 27, 2012
    Should be (myClass.Process(input))