Correct way to perform asynchronous operations in C#

Question

I've got a class that takes for ages to spin up due to it performing some long running operations in the constructor. I need to create 3 of these objects and add them to a dictionary.

I'm trying to make use of async so that I create 3 threads, each spinning up one of these objects, but finding it a little confusing.

Initially I thought I'd wait until all threads were completed, and then add the results to the dictionary, but I couldn't get the code to compile, so I've resorted to doing the dictionary.Add within the thread (is this ok?)

I've written the following code

public void CreateAccountStuffs(ICollection<Account> accounts)
{
    CreateStuff(accounts);
    
}

        
private async void CreateStuff(ICollection<Account> accounts)
{
     var tasks = accounts.Select(a => CreateStuffForAccount(a));
     await Task.WhenAll(tasks);
}

private Task CreateStuffForAccount(Account account)
{
        //Long Running process due to website calls in AccountWrapper construction
        var accountWrapper = new AccountWrapper(account);
                
        
        return Task.FromResult(accountWrapper);
}

The final line seems pointless, but I can't get the code to compile without it. I feel like I'm missing something really obvious here, but my multithreading skills are so rusty I'm struggling to see what.

My gut feeling is that there is a far simpler way of writing what I need to do, but I don't know what that is.

e.g. I notice this question appears to suggest that you can write a method that returns Task and simply in the method body write return "This String", however, that construct doesn't compile for me. I get the error cannot convert from string to Task

Note - Edited to remove code not part of the problem - there were 2 constructors involved before, which confused things - it's the constructor in AccountWrapper that is long running.

You don't have any async code. Simply marking a method with `async` doesn't make it asynchronous. You can run non-async code in parallel with `Task.Run(() => ....` — Crowcoder, Jul 03 '20 at 11:41
You need to do your long running tasks in an async way, but thats not possible in the constructor. perhaps that clears some thins for you: https://stackoverflow.com/questions/23048285/call-asynchronous-method-in-constructor Or this: https://blog.stephencleary.com/2013/01/async-oop-2-constructors.html — Homungus, Jul 03 '20 at 11:44
@Homungus The second of those is useful thanks - I'll refactor to use the pattern suggested there and see where I end up. — tonydev314, Jul 03 '20 at 11:51
I've refactored the AccountWrapper class to use a factory constructor public static Task CreateAsync(Account account) however, now in the calling code, if I try to access the AccountWrapper - it tells me it can't convert from Task to AccountWrapper and I should use await, but if I put await in THIS code, then I need to make that async as well! — tonydev314, Jul 03 '20 at 14:32

Ilnar Gabidullin · Accepted Answer · 2020-07-03T12:04:45.503

It is not good design to place async method in constructor and do not await it. Becuse when you code is return from constructor, you do not now if the construction have been finished and you class instance may be in inconsistent state. You can simply create static factory method

public class MyClass
{
    private readonly IDictionary<int, IAccountStuff> _accountStuffs;
    private readonly ICollection<Account> _accounts;

    protected MyClass()
    {
        _accounts = dataLayer.GetAccounts();
        // remove it
        // CreateStuff(accounts);
    }

    private async void CreateStuff(ICollection<Account> accounts)
    {
        var tasks = accounts.Select(a => CreateStuffForAccount(a));
        await Task.WhenAll(tasks);
    }

    private Task CreateStuffForAccount(Account account)
    {
        //Long Running process due to website calls in AccountWrapper construction
        return Task.Run(async () =>
        {
            // do something with you account
            // to do this you dictionary should be concurrent
            this._accountStuffs.Add(account.Id, accountWrapper.Stuff);
        });
    }

    // factory method
    public static async Task Create()
    {
        var myClass = new MyClass();
        await myClass.CreateStuff(); 
    }
}


....
// and somwhere you may use
public async Task Foo()
{
    var myClass = await MyClass.Create();
    // now you can safely use you class instance myClass 
}

score 1 · Answer 2 · answered Jul 03 '20 at 12:15

It is important to keep in mind the difference between parallelism and asynchronicity. The former is meant to use multiple cores for better performance, the later is meant to hide latency, especially while doing IO operations. It is not clear what you are going for in this case.

you use Task.FromResult(accountWrapper), this just creates a task that is already completed, so no async is going on here. This is meant for when a synchronous method need to fulfill a asynchronous interface, for example if the result is cached, so it can return directly.

To create the objects in parallel you could use Parallel.For or Parallel.Foreach to create all accountWrappers in parallel. But this would not use async, i.e. the call would block until all accountwrappers has been created.

To create your class asynchronously you would use

Task.Run(() => new MyClass())

This will create the class on a background thread and return a task that completes when this has been done. Use await to get the result from a task. Keep in mind that that result can be an exception. You typically want to avoid async void so the caller can handle these exceptions. See best practices.

You can combine both methods, i.e. Create your class on a background thread, and the constructor can use Parallel.For/Foreach to create the expensive AccountWrappers.

Correct way to perform asynchronous operations in C#

2 Answers2