The reason why you shouldn’t use ToList() in a single Lambda Expression

One of the most frequent unnecessary issues I see introduced into a client’s code base, is over the usage of ToList(). It’s such a simple extension but so many developers balls it up really badly. Here is an obvious example in question:

[TestMethod]
public void Test_Enumerable_To_List()
{
var List = new List<ListObject>();
IEnumerable<string> enumerable = null;
var result = enumerable.Select(x => new ListObject
{
Title = x
}).ToList();
List = result;
}

Hopefully, this bit of code should make it really obvious what’s happening. The variable enumerable is null so calling the ToList() extension on it will cause a null argument exception. You might be reading this thinking no one would ever do that… but a more common real world scenario would be:

public List<ListObject> ConvertToListObject(IEnumerable<string> enumerable)
{
return enumerable.Select(x => new ListObject
{
Title = x
}).ToList();
}

The biggest excuse I hear for people writing this type of code is that the select can never return null so the ToList() will never throw. I find it very frustrating that people don’t get that if the underlining data source is null the ToList() will throw.

In some scenarios, you may never want to do a null conversation on an empty enumerable. In those situations there are a plethora of showing your intention in your code. Purposely check for null and throw, log it etc.. when you leave these types of bugs in a real world situation it can take hours to track down if the enumerable is only null state infrequently.

If you don’t need a List don’t use it!

The biggest culprit for this is usually returning List when they don’t need to. A lot of developers that I have mentored love to return a List in their methods return signature even when it’s never needed. My advise is to always stick with an IEnumerable in the first place and you would have avoided the issue entirely.

Don’t do the ToList() on the Select

If you want/have to use ToList() don’t do it all in one single assignment. Do the lambda in one line and then do the null check and conversation on another. it really is that simple

public List<ListObject> ConvertToListObject(IEnumerable<string> enumerable)
{
var result = enumerable.Select(x => new ListObject
{
Title = x
});
return result != null ? result.ToList() : null;
}

User a custom extensions

If you don’t like that approach, you can minimize your duplicate code with an extension(s).

public static bool IsNullOrEmpty<T>(this IEnumerable<T> enumerable)
{
return enumerable == null || !enumerable.Any();
}

With the code now looking like this:

public List<ListObject> ConvertToListObject(IEnumerable<string> enumerable)
{
var result = enumerable.Select(x => new ListObject
{
Title = x
});
result.IsNullOrEmpty() ? result.ToList() : null;
}

ToListNullSafe

Lastly if you don’t like any of these approach you can create your own null safe ToList() extension.

public static List<T> ToListNullSafe<T>(this IEnumerable<T> source)
{
return source.IsNullOrEmpty() ? source.ToList() : null;
}

The code would now look like this:

public List<ListObject> ConvertToListObject(IEnumerable<string> enumerable)
{
return enumerable.Select(x => new ListObject
{
Title = x
}).ToListNullSafe();
}

If you do a lot of peer reviews, frequently pointing out the same thing repeatedly gets tiresome quickly. In this case when a bug is so simple to never introduce into your code base it should be easy to prevent.

Jon D Jones

Software Architect, Programmer and Technologist Jon Jones is founder and CEO of London-based tech firm Digital Prompt. He has been working in the field for nearly a decade, specializing in new technologies and technical solution research in the web business. A passionate blogger by heart , speaker & consultant from England.. always on the hunt for the next challenge

More Posts

3 replies
  1. Scott Reed
    Scott Reed says:

    The main benefit really with using IEnumerable collections as return methods is the deferred execution and the benefit to the compiler.They can be chained and at the point of binding the data or converting it to an list/array the compiler will optimize the query against all the chained LINQ statements. I always find it a good pattern to make as much as possible, although sometime it’s a necessity to tolist/toarray it so the execution happens at that point. Else you can find values being different from what the code expected do to it not be evaluated when I was expecting

    Reply
  2. Björn Ali Göransson
    Björn Ali Göransson says:

    If you want to check for null before doing ToList(), and return null if it was null, you can do this in C#6 (only available in VS 2015+ afaik):

    return source?.ToList();

    Reply

Leave a Reply

Want to join the discussion?
Feel free to contribute!

Leave a Reply

Your email address will not be published. Required fields are marked *