Click here to Skip to main content
15,885,365 members
Articles / Ienumerable

A behavioural difference between IEnumerable.ForEach and List.ForEach

Rate me:
Please Sign up or sign in to vote.
4.50/5 (2 votes)
14 Dec 2015CPOL2 min read 8.5K   1   2
This morning at work a colleague and myself discovered a slightly alarming difference between `IEnumerable.ForEach` and `List.ForEach` when we switched a collection type for a field in a C# ASP.NET MVC project.

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.
 

// --------------------------------------------------------------------------------------------------------------------
// <copyright file="ListExtensions.cs" company="Microsoft">
//   Copyright Microsoft Corporation, all rights reserved
// </copyright>
// <summary>
//   ListExtensions Class - Provides the extension on List
// </summary>
// --------------------------------------------------------------------------------------------------------------------

namespace WebGrease.Css.Extensions
{
    using System;
    using System.Collections.Generic;
    using System.Collections.ObjectModel;
    using System.Linq;

    /// <summary>ListExtensions Class - Provides the extension on List</summary>
    public static class ListExtensions
    {
    
        /// <summary>For each extension method for IEnumerable</summary>
        /// <typeparam name="T">The type of items in collection</typeparam>
        /// <param name="list">The list of items</param>
        /// <param name="action">The action to perform on items</param>
        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
{
    /// <summary>Represents a strongly typed list of objects that can be accessed by index. Provides methods to search, sort, and manipulate lists.</summary>
    /// <filterpriority>1</filterpriority>
    [DebuggerDisplay("Count = {Count}")]
    [DebuggerTypeProxy(typeof(Mscorlib_CollectionDebugView<>))]
    [Serializable]
    public class List<T> : IList<T>, ICollection<T>, IEnumerable<T>, IList, ICollection, IEnumerable
    {

        /// <summary>Performs the specified action on each element of the <see cref="T:System.Collections.Generic.List`1"></see>.</summary>
        /// <param name="action">The <see cref="T:System.Action`1"></see> delegate to perform on each element of the <see cref="T:System.Collections.Generic.List`1"></see>.</param>
        /// <exception cref="T:System.ArgumentNullException">action is null.</exception>
        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...

License

This article, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)


Written By
Software Developer
United Kingdom United Kingdom
Duane has worked in a commercial software development environment for 9 years, with all but three for a global fashion retailer.

He is proficient in ASP.Net, MVC, C#, HTML, CSS, JavaScript, SQL Server TSQL.

Comments and Discussions

 
QuestionHiding Exceptions is Too Common Pin
Scott Emberley15-Dec-15 8:47
Scott Emberley15-Dec-15 8:47 
PraiseRe: Hiding Exceptions is Too Common Pin
dibley197315-Dec-15 9:03
dibley197315-Dec-15 9:03 
Yes, I fear you are correct. What is more that is in Microsoft code too, which should be an example to follow!

General General    News News    Suggestion Suggestion    Question Question    Bug Bug    Answer Answer    Joke Joke    Praise Praise    Rant Rant    Admin Admin   

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.