Difference between .ForEach(async x=> {await}) and foreach{await} in an asnyc method

Question

What is the difference between these two simplified pieces of code, specifically the foreach parts.

public async Task UploadFilesAsync(IEnumerable<UploadFile> uploads)
{
    uploads.ToList().ForEach(async upload => 
    {
     await UploadFileAsync(upload);
    });
}

public async Task UploadFilesAsync(IEnumerable<UploadFile> uploads)
{
    foreach (upload in uploads)
     {
      await UploadFileAsync(upload);
     }
}

I know the first essentially creates an async void action which is not the greatest solution for various reasons, but would the second do the same, or is that more of an accepted practice?

https://stackoverflow.com/questions/18667633/how-can-i-use-async-with-foreach — CodeCaster, Oct 06 '17 at 12:51
https://stackoverflow.com/questions/40091686/asynchronous-foreach — CodeCaster, Oct 06 '17 at 12:52
I've read pretty much every SO question on this subject including those 2, I didn't see an exact answer for this though, regardless off the right or wrongness of using this approach — mameesh, Oct 06 '17 at 12:58
@mameesh Well the linked question specifically explains what the difference is between how the two behave, so if you read it, then you *do* know what the difference is between the two. — Servy, Oct 06 '17 at 13:27

Daniel Leiszen · Answer 1 · 2017-10-07T08:26:58.877

Since the first method includes a foreach that is not part of the async execution it will start the inner anonymous methods and quit. The anonymous methods would finish after the UploadFileAsync method finished.

The second version runs sequentially because the foreach awaits each iteration (before calling MoveNext()), so the UploadFileAsync method would quit after all the inner calls finished.

You can test it with the following code:

    class Program
{
    static void Main(string[] args)
    {
        int[] uploads = new int[] { 600, 2000, 1000 };

        UploadFilesAsync(uploads).ConfigureAwait(true);
        UploadFilesAsync2(uploads).ConfigureAwait(true);

        Console.ReadLine();
    }

    public static async Task UploadFilesAsync(IEnumerable<int> uploads)
    {
        Console.WriteLine("Start version 1");

        uploads.ToList().ForEach(async upload =>
        {
            Console.WriteLine("Start version 1 waiting " + upload);

            await Task.Delay(upload);

            Console.WriteLine("End version 1 waiting " + upload);
        });

        Console.WriteLine("End version 1");
    }

    public static async Task UploadFilesAsync2(IEnumerable<int> uploads)
    {
        Console.WriteLine("Start version 2");

        foreach (var upload in uploads)
        {
            Console.WriteLine("Start version 2 waiting " + upload);

            await Task.Delay(upload);

            Console.WriteLine("End version 2 waiting " + upload);
        }

        Console.WriteLine("End version 2");
    }
}

So, this is not a matter of right or wrong. It is simply about what you would like to achieve. Using the first version you can easily face a closure problem, if you have resources created in the UploadFilesAsync method and want to use them in the inner anonymous methods. Since the method quits before the uploads finish, the resources you created would be freed before the inner methods could use them.

`The second version runs synchronosly` No, it runs *sequentially*. It runs asynchronously, it's just that each asynchronous operation isn't started until the previous one has finished. — Servy, Oct 06 '17 at 13:22
`So, this is not a matter of right or wrong.` Even if you *do* just want to fire off all of the operations and then forget about them, it's a very poor way of doing it, in that it's not obvious that that's is your intention. In fact, it looks like they're *trying* to run the operations sequentially, they just failed to do so properly. If they *did* want to fire them all and forget them, they should write the code such that that intention is clear. I would consider it a mistake otherwise. — Servy, Oct 06 '17 at 13:24

Vikhram · Answer 2 · 2017-10-06T13:19:51.457

First things first. Your focus seems to be on the async/await part of the code, so I will answer that first.

I don't believe there is any difference at all from that perspective

But other than that there are some interesting differences

By calling uploads.ToList() in approach #1, you are iterating over the complete IEnumerable<UploadFile> uploads whereas in the approach #2, you are using the power of late execution
By using List<T>.ForEach, you necessitate the creation of a lambda expression whose body will get invoked for each element in the List<T>. This can be fairly expensive if it is invoked a lot of times and does fairly light weight work. You clearly avoid this unwanted overhead in approach #2
Exception handling will be a pain in approach #1 whereas approach #2 allows for the use for typical exception handling mechanisms

Now, understand that when your code hits the await call, it pretty much gets suspended for the Task to get completed. Unless you want to process the results of UploadFileAsync(upload) after that call, you can simply proceed without await'ing and let the execution continue, till the point where these results are needed

public async Task UploadFilesAsync(IEnumerable<UploadFile> uploads)
{
    foreach (upload in uploads)
    {
        UploadFileAsync(upload);
    }
}

This way, you can let the caller decide, when to wait for the results.

Difference between .ForEach(async x=> {await}) and foreach{await} in an asnyc method

2 Answers2