This morning at work a colleague and myself discovered a slightly alarming difference between `IEnumerable<T>.ForEach
` and `List<T>.ForEach
` when we switched a collection type for a field in a C# ASP.NET MVC project. Initially the field type had been `IEnumerable<SomeType>
` and we had been calling the `.ForEach
` on it passing in an action. This was using the `IEnumerable<T>.ForEach
` which is defined in the `WebGrease.Css.Extensions
` namespace as we have WebGrease already referenced. When we switched the field type to `List<SomeType>
` this line auto resolved (maybe via Resharper) to the `.ForEach` within the `System.Collections.Generic
` namespace. We started getting some null reference exceptions in our tests.
What we found was for most code-paths this was not a problem, however when the field was null because the WebGrease version is an extension method (see code below) the method checks for null and then just bugs out of the method with no action being taken.
namespace WebGrease.Css.Extensions
{
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
public static class ListExtensions
{
public static void ForEach<T>(this IEnumerable<T> list, Action<T> action)
{
if (list == null || action == null)
{
return;
}
foreach (var item in list)
{
action(item);
}
}
}
}
In the List<T>
version in `System.Collections.Generic
` the ForEach
is an instance method so there is no way the method can be called on a null instance, hence the null reference exception. The decompiled code for the method is below.
namespace System.Collections.Generic
{
[DebuggerDisplay("Count = {Count}")]
[DebuggerTypeProxy(typeof(Mscorlib_CollectionDebugView<>))]
[Serializable]
public class List<T> : IList<T>, ICollection<T>, IEnumerable<T>, IList, ICollection, IEnumerable
{
public void ForEach(Action<T> action)
{
if (action == null)
{
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.match);
}
for (int i = 0; i < this._size; i++)
{
action(this._items[i]);
}
}
}
}
So my beef here is with the authors of the WebGrease assembly. If they had maybe put a guard on the `list` parameter to throw an exception so it follows closer to the List<T>
implementation, rather than just "gracefully" eating the issue and carrying on with no action taken, we would have guarded against a null list ourselves a bit earlier in the development. Or maybe my gripe is more with the fact that extension methods can be called on a `null` object!
I'm now left wondering if any of my other software maybe out there with a little time-bomb like this waiting to blow...