35

I have the following code that correctly uses async/await paradigm.

internal static async Task AddReferencseData(ConfigurationDbContext context)
{
    foreach (var sinkName in RequiredSinkTypeList)
    {
        var sinkType = new SinkType() { Name = sinkName };
        context.SinkTypeCollection.Add(sinkType);
        await context.SaveChangesAsync().ConfigureAwait(false);
    }
}

What is the equivalent way to write this if, instead of using foreach(), I want to use LINQ ForEach()? This one, for example, gives compile error.

internal static async Task AddReferenceData(ConfigurationDbContext context)
{
    RequiredSinkTypeList.ForEach(
        sinkName =>
        {
            var sinkType = new SinkType() { Name = sinkName };
            context.SinkTypeCollection.Add(sinkType);
            await context.SaveChangesAsync().ConfigureAwait(false);
        });
}

The only code I got to work without compile error is this.

internal static void AddReferenceData(ConfigurationDbContext context)
{
    RequiredSinkTypeList.ForEach(
        async sinkName =>
        {
            var sinkType = new SinkType() { Name = sinkName };
            context.SinkTypeCollection.Add(sinkType);
            await context.SaveChangesAsync().ConfigureAwait(false);
        });
}

I'm worried that this method has no async signature, only the body does. Is this the correct equivalent of my first block of code above?

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
SamDevx
  • 2,268
  • 4
  • 32
  • 47
  • 9
    `ForEach` not a linq function – Grundy May 15 '15 at 13:35
  • 2
    Why do you want to change it at all? If it ain't broken, don't fix it. – H H May 15 '15 at 13:38
  • 1
    Excelent post from Eric Lipert using “foreach” vs “ForEach” http://blogs.msdn.com/b/ericlippert/archive/2009/05/18/foreach-vs-foreach.aspx – Deepu Madhusoodanan May 15 '15 at 13:40
  • 3
    If I was going to do anything, I'd put the `SaveChangesAsync` outside the loop. I doubt there's any advantage to putting it inside (on the contrary, there's probably a performance penalty). – Charles Mager May 15 '15 at 13:41
  • 1
    @Charles Mager indeed - it's almost always a bad idea to put ``SaveChanges()`` in a loop. On the other note, why are you invoking the async version exactly? – Jacek Gorgoń May 15 '15 at 13:58
  • 1
    The reference to Eric Lipert's blog doesn't seem to be valid anymore. Here's a valid link to his website for what should be the same article: https://ericlippert.com/2009/05/18/foreach-vs-foreach/ – Bob Gear Feb 03 '22 at 17:38

6 Answers6

48

No. It isn't. This ForEach doesn't support async-await and requires your lambda to be async void which should only be used for event handlers. Using it will run all your async operations concurrently and won't wait for them to complete.

You can use a regular foreach as you did but if you want an extension method you need a special async version that iterates over the items, executes an async operation and awaits it.

However, you can create one:

Staring from .NET 6.0 you can use Parallel.ForEachAsync:

public async Task ForEachAsync<T>(this IEnumerable<T> enumerable, Func<T, Task> action)
{
    await Parallel.ForEachAsync(
        enumerable,
        async (item, _) => await action(item));
}

Or, avoid the extension method and just call it directly:

await Parallel.ForEachAsync(
    RequiredSinkTypeList,
    async (sinkName, _) =>
    {
        var sinkType = new SinkType() { Name = sinkName };
        context.SinkTypeCollection.Add(sinkType);
        await context.SaveChangesAsync().ConfigureAwait(false);
    });

On older platforms you need to use foreach:

public async Task ForEachAsync<T>(this IEnumerable<T> enumerable, Func<T, Task> action)
{
    foreach (var item in enumerable)
    {
        await action(item);
    }
}

Usage:

internal static async Task AddReferencseData(ConfigurationDbContext context)
{
    await RequiredSinkTypeList.ForEachAsync(async sinkName =>
    {
        var sinkType = new SinkType() { Name = sinkName };
        context.SinkTypeCollection.Add(sinkType);
        await context.SaveChangesAsync().ConfigureAwait(false);
    });
}

A different (and usually more efficient) implementation of ForEachAsync would be to start all the async operations and only then await all of them together but that's only possible if your actions can run concurrently which isn't always the case (e.g. Entity Framework):

public Task ForEachAsync<T>(this IEnumerable<T> enumerable, Func<T, Task> action)
{
    return Task.WhenAll(enumerable.Select(item => action(item)));
}

As was noted in the comments you probably don't want to use SaveChangesAsync in a foreach to begin with. Preparing your changes and then saving them all at once will probably be more efficient.

i3arnon
  • 113,022
  • 33
  • 324
  • 344
  • @i3amon thanks i like your last suggestion. If "SinkTypeCollection" is of thread safe type, would that work? – SamDevx May 15 '15 at 14:06
  • @SamDevx actually, it would also work if it isn't as it's in the synchronous part of the async lambda. The issue it really in `SaveChangesAsync`. – i3arnon May 15 '15 at 14:09
3

.Net 6 now has

 Parallel.ForEachAsync

used like

await Parallel.ForEachAsync(userHandlers, parallelOptions, async (uri, token) =>
{
    var user = await client.GetFromJsonAsync<GitHubUser>(uri, token);
 
    Console.WriteLine($"Name: {user.Name}\nBio: {user.Bio}");
});

https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.parallel.foreachasync https://www.hanselman.com/blog/parallelforeachasync-in-net-6

Murdock
  • 4,352
  • 3
  • 34
  • 63
1

The initial example with foreach effectively waits after each loop iteration. The last example invokes List<T>.ForEach() that takes an Action<T> meaning, that your async lambda will compile into a void delegate, as opposed to standard Task.

Effectively, the ForEach() method will invoke "iterations" one by one without waiting for each one to finish. This will also propagate to your method, meaning that AddReferenceData() might finish before the work is done.

So no, it's not an equivalent and behaves quite differently. In fact, assuming it's an EF context, it will blow up as it may not be used across multiple threads concurrently.

Also read http://blogs.msdn.com/b/ericlippert/archive/2009/05/18/foreach-vs-foreach.aspx mentioned by Deepu for why it's probably better to stick to foreach.

Jacek Gorgoń
  • 3,206
  • 1
  • 26
  • 43
0

To write await in a method your method needs to be marked as async. When you write ForEach method you are writing await inside your labda expression which is in terms different method altogether which you are calling from your method. You need to move this lamdba expression to method and mark that method as async and also as @i3arnon said You need async marked ForEach method which is not yet provided by .Net Framework. So you need to write it on your own.

Pramod Jangam
  • 316
  • 3
  • 10
0

Thanks all for your feedback. Taking the "save" part outside the loop, I believe the following 2 methods are now equivalent, one using foreach(), another using .ForEach(). However, as mentioned by Deepu, I will read Eric's post on why foreach might be better.

public static async Task AddReferencseData(ConfigurationDbContext context)
{
    RequiredSinkTypeList.ForEach(
        sinkName =>
        {
            var sinkType = new SinkType() { Name = sinkName };
            context.SinkTypeCollection.Add(sinkType);
        });
    await context.SaveChangesAsync().ConfigureAwait(false);
}

public static async Task AddReferenceData(ConfigurationDbContext context)
{
    foreach (var sinkName in RequiredSinkTypeList)
    {
        var sinkType = new SinkType() { Name = sinkName };
        context.SinkTypeCollection.Add(sinkType);
    }
    await context.SaveChangesAsync().ConfigureAwait(false);
}
MJK
  • 3,434
  • 3
  • 32
  • 55
SamDevx
  • 2,268
  • 4
  • 32
  • 47
-1

Why not use AddRange() method?

context.SinkTypeCollection.AddRange(RequiredSinkTypeList.Select( sinkName  => new SinkType() { Name = sinkName } );

await context.SaveChangesAsync().ConfigureAwait(false);
Tzu
  • 235
  • 1
  • 9