CODEWITHSUNDEEP
c#
Mike Lowery
Mike Lowery

Reputation: 2868

ForEach extension method not being called with Dictionary

I've got a Dictionary that contains a ValueTuple:

    private static readonly Dictionary<RELAY, (byte RelayNumber, bool IsClosed)> s_relays = new Dictionary<RELAY, (byte RelayNumber, bool IsClosed)>
    {
        { RELAY.OUTPUT1, (4, false) },
        { RELAY.OUTPUT2, (9, false) },
        { RELAY.OUTPUT3, (11, false) },
    };

Later in my code I set IsClosed to true for one or more relays. I wrote a ForEach extension method for the Dictionary:

    public static IEnumerable<T> ForEach<T>(this IEnumerable<T> enumeration, Action<T> action)
    {
        Debug.Assert(enumeration != null, $"Argument {nameof(enumeration)} cannot be null!");
        Debug.Assert(action != null, $"Argument {nameof(action)} cannot be null!");

        foreach (T item in enumeration)
        {
            action(item);
            yield return item;
        }
    }

I want to call a method for each relay that is closed. For example, writing to Console.WriteLine:

Relays.Where(x => x.Value.IsClosed).ForEach(x => Console.WriteLine(x.Key));

But this does not work. However, if I include ToList() the code below does work:

Relays.Where(x => x.Value.IsClosed).ToList().ForEach(x => Console.WriteLine(x.Key));

What am I missing?

(I do realize that the ForEach extension method being called is different between these two examples with the first one [mine] never being called.)

Upvotes: 3

Views: 185

Answers (2)

CodingYoshi
CodingYoshi

Reputation: 27019

It is just a query and it will only be executed "lazily" when needed. Let's make things simple so we can see what happens:

private static readonly List<int> nums = new List<int> { 1, 2, 3 };

public static IEnumerable<int> MyEach(this IEnumerable<int> enumeration, Action<int> action)
{ 
    foreach (var item in enumeration)
    {
        action(item);
        yield return item;
    }
}

This is just a query, so nothing will execute yet:

var query = nums.Where(x => x == 1).MyEach(x => Console.WriteLine(x));

But as soon as you do this:

query.ToList();

It will execute MyEach extension method.

You have to be careful because it depends when it gets executed. For example, try this and you will see the number 1 printed to the console 3 times:

var query = nums.Where(x => x == 1).MyEach(x => Console.WriteLine(x));
nums.Add(1);
nums.Add(1);
query.ToList();

However, if you removed the yield, then it will execute right away:

public static void MyEach(this IEnumerable<int> enumeration, Action<int> action)
{ 
    foreach (var item in enumeration)
    {
        action(item);
        //yield return item;
    }
}

// Now it will execute right away
nums.Where(x => x == 1).MyEach(x => Console.WriteLine(x));

Upvotes: 1

David L
David L

Reputation: 33833

In this case, unlike the List<T>.ForEach() implementation that iterates a collection, by using the yield return pattern, your .ForEach implementation is actually a deferred execution "filter" that will not be executed until the query is fully resolved.

In that sense it is extremely similar to the .Where() method except that instead of reducing the query set, it is performing a side-effect operation on it as the query is resolved. Neither will actually be executed until the enumeration is enumerated.

Adding .ToList() to the end of your query will execute your method as expected and resolve the query.

Alternatively, if you simply want a convenient way to iterate and perform an action on a collection, you could remove the yield return and simply iterate the collection and return it at the end:

public static IEnumerable<T> ForEach<T>(this IEnumerable<T> enumeration, Action<T> action)
{
    Debug.Assert(enumeration != null, $"Argument {nameof(enumeration)} cannot be null!");
    Debug.Assert(action != null, $"Argument {nameof(action)} cannot be null!");

    foreach (T item in enumeration)
    {
        action(item);
    }

    return enumeration;
}

However, the risk there is that you could potentially enumerate twice.

It is also worth noting that your second example

Relays.Where(x => x.Value.IsClosed).ToList().ForEach(x => Console.WriteLine(x.Key));

does not actually invoke your extension method. Rather, it invokes the List<T>.ForEach() method.

Upvotes: 3

Related Questions