C# IEnumerable, ICollection, IList or List?

Question

Can somebody explain to me why we constantly use IEnumerable all over C# when, by my logic we should be using List,IList or ICollection instead.

Look at this commit fix.

Ok, number 1) IEnumarable should only be used for read only collections!

Great, everybody keeps repeating that (I get it), so why do I need to constantly need putting ToList() on every single database call to prevent mulit-threads creashing when the original thread disposes of context.

Surely once the query has been Enumarated to a List, we should just use List from there on to avoid confusion, thread safety issues and constantly calling ToList() on every single model, over and over.

Also on WebAPI that parse JSON, a text representation of an already enumerated object, we store it as IEnumarable again. We already know the size, we not interested in lazy loading it, we probably going to modify that object somewhere along the line, so why don't we store it as List of.

--EDIT

One important aspect about this question is that I never knew about covariance and contravariance

You cannot do List<IItem> because of runtime problems on the List implementation so you have to use IEnumarable<IItem> - This will only make sense in specific cases where you have the same entity, on two different systems, that need to work on one piece of code. Otherwise I have been using List<> since asking the question and that is probably 90% of the time correct to use in business layers. Database/Repository layers stick to IColletion now or IQueryable.

Answer 1

It's kinda hard to answer this because it's very easy to post an opinion, and not an answer. I'll try though.

This is a fundamental question about responsibility, more specifically, good API design.

Here are my thoughts about the specific corner of the world of API design that this covers:

• Be as specific as you can in your output types
• Be as open as you can in your input types

I'll give a few examples. If your method can take any collection, take IEnumerable<T>. If your method needs an indexable collection, take IList<T> or similar. If the first thing the method does is convert the input to a List through .ToList(), be open about it. If I already have a list on the outside and I want to give it to your method, take List<T>.

However, I must be aware of the fact that once you say you take IList<T>, you're able to modify the list. If that is OK, good, I can simply send you my list. Otherwise, I will have to execute .ToList() on the outside. This is not worse than before, it's simply a question about who is responsible for doing so.

On the opposite side, if your method returns an object that is a List<T>, return it as List<T> or IList<T>. Don't hide it behind IEnumerable<T>.

Here you should also be aware that any caller that gets this list back is able to modify it. If this is not OK, don't return it as IList<T>.

Though, you can say in that last example that if that list is not supposed to be modified on the outside, after returning (it's not the callers list), then you should hide it behind IEnumerable<T>, but this only hides the fact, the programmer can still cast it back to IList<T> and modify it. If you can't accept this, return a copy through .ToList().

However, all of this goes back to responsibility.

If your method returns IEnumerable<T>, shouldn't I, the person writing code that is using this method, rely on your method working correctly? If your method won't work because you've already disposed of the database context, then that's on you, your code, your bug, your problem.

Likewise, if I need something that can be indexed into, but other than that is a purely readonly collection, I should design my method to take the appropriate collection interface to signal this. If I design my method to take IEnumerable<T>, I should know that I'm specifically saying "I'll take any lazily produced collection". If I don't know that, or don't want to say that, then I should not take IEnumerable<T>.

So as to answer the underlying question for your specific line of code. Here you absolutely must call .ToList() inside because otherwise your methods is broken, buggy, wrong.

OR, you can redesign your method in such a way that it retains its lazy nature, but doesn't dispose of the database context until it's ready. A simple change to your method in this regard would be to use yield return:

using (var context = ... )
{
    foreach (var element in context.Request(...))
        yield return element;
}

With such a change you're still postponing the actual cost of going to the database until the caller actually iterates over the collection you returned, and you're still disposing of the context correctly (assuming the caller does the iteration correct). Here I would still return IEnumerable<T> to specifically say "I'm lazily evaluated".

However, if the list that you produce internally is not kept, nobody is responsible for it, you're effectively giving the responsibility of this list to the caller, then absolutely should you return it as List<T>. Don't hide it behind IEnumerable<T>.