How(and why) can I avoid returning a void on these async methods?

Question

EDIT: So it seems having the method return void instead of task means that exceptions are propagated on the wrong(unexpected?) context. However, my IDE (Xamarin) is still kicking up a fuss on the line in my constructor where I call AttemptDatabseLoad()

"The statement is not awaited and execution of current method continues before the call is completed. Consider using 'await' operator or calling 'Wait' method"

Why is it kicking up this fuss? Surely the entire purpose of using an async method is precisely so that the program continues execution on the Main thread.

I've read a fair bit on async and await as I need to have some async data loading for an app i'm making. I've read in loads of places that it's bad practice to have an async method return void (excepting in cases of firing events) and I understand the reason why it can be good to keep a handle on the Task. However, I can't see anything logically wrong with what I've written below so my question is twofold: Why is my current code poor practice? How should it be re-written?

private const int MAX_CONNECTION_ATTEMPTS = 10;
private int ConnectionAttempts = 0;

//Constructor
public DataLoader()
{
    //First load up current data from local sqlite db
    LoadFromLocal();

    //Then go for an async load from 
    AttemptDatabaseLoad();
}

public async void AttemptDatabaseLoad()
{
    while(ConnectionAttempts < MAX_CONNECTION_ATTEMPTS){
        Task<bool> Attempt = TryLoad ();
        bool success = await Attempt;
        if (success) {
            //call func to load data into program memory proper
        }else{
            ConnectionAttempts++;
        }
    }
}

//placeholder for now
public async Task<bool> TryLoad()
{
    await Task.Delay(5000);
    return false;
}

Answer 1

Constructor are meant to bring an object to it's fully constructed structure once initialized. On the other hand, async methods and constructors don't play well together, as a constructor is inherintly synchronous.

The way to get around this problem is usually to expose an initialization method for the type, which is itself async. Now, you let the caller initialize the object fully. Note this will require you to monitor the actual initialization of the method.

Async shines when you need scale. If you don't expect this to be an IO bottleneck in your application, perhaps consider using synchronous methods instead. This will give you the benefit of actually fully initializing your object once the constructor finishes execution. Although, I don't think I would initiate a call to a database via a constructor anyway:

public async Task InitializeAsync()
{
    LoadFromLocal();
    await AttemptDatabaseLoadAsync();
}

public async Task AttemptDatabaseLoadAsyncAsync()
{
    while(ConnectionAttempts < MAX_CONNECTION_ATTEMPTS)
    {
        Task<bool> Attempt = TryLoad ();
        bool success = await Attempt;
        if (success)
        {
            //call func to load data into program memory proper
        }
        else
        {
            ConnectionAttempts++;
        }
    }
}

And call it:

var dataLoader = new DataLoader();
await dataLoader.InitializeAsync();

Answer 2

I understand the reason why it can be good to keep a handle on the Task.

So it seems having the method return void instead of task means that exceptions are propagated on the wrong(unexpected?) context.

One of the reasons it's good to have a Task is that you can use it to retrieve the results of the asynchronous method. And by "results", I don't just mean the return value - I mean exceptions, too. The Task represents the execution of that asynchronous method.

When an exception escapes an async Task method, it's placed on that returned task. When an exception escapes an async void method, there's no obvious place for it to go, so the actual behavior is to raise it directly on the SynchronizationContext that was current at the beginning of the async void method. This sounds odd, but it's specifically designed to emulate exceptions escaping event handlers.

Of course, if your async void method isn't an event handler (like this example), then the behavior seems very odd and surprising.

Why is it kicking up this fuss? Surely the entire purpose of using an async method is precisely so that the program continues execution on the Main thread.

I think you're misunderstanding the warning message. Since the Task represents the execution of that method, to ignore it is an error 99.9% of the time. By ignoring it, your code is saying that it doesn't care when the async method completes, what its return value is (if any), and whether or not it throws exceptions. It's extremely rare for code to not care about any of these.

How should it be re-written?

I have a blog post on how to do "async constructors". My favorite approach is the asynchronous factory method:

//Constructor
private DataLoader()
{
  //First load up current data from local sqlite db
  LoadFromLocal();
}

public static async Task<DataLoader> CreateAsync()
{
  var result = new DataLoader();
  await result.AttemptDatabaseLoadAsync();
  return result;
}

However, since you're using this in a UI application, I suspect you'll eventually run into a situation where you want to call asynchronous code from your ViewModel constructor. Asynchronous factories are great for helper code (like DataLoader), but they don't work for ViewModels because the VMs need to be created immediately - the UI needs to show something now.

At the UI layer, you have to first initialize your UI to some kind of a "loading" state, and then update it to a "normal" state once the data has arrived. I prefer to use asynchronous data binding for this, as described in my MSDN article.

Answer 3

You can just change return type to Task (non-generic) and don't return "explicitly" from async method. The reasons why it's better to use void only on top-level functions can be found here: async/await - when to return a Task vs void? So, it's mainly about recovery from exception in your async-void method. I hope it will help.

EDIT: One more thing - because I didn't notice that you're calling it from constructor. Please check also this answer: https://stackoverflow.com/a/23051370/580207 and this blog post: http://blog.stephencleary.com/2013/01/async-oop-2-constructors.html

Answer 4

Why is my current code poor practice?

The callers of DataLoader() constructor might experience the following issues:

• Code instantiating DataLoader class is not aware that load operation is still in progress after DataLoader() returns, so it cannot use the data retrieved by async AttemptDatabaseLoad().

• There is no way to discover when the loaded data becomes available.

• It cannot be composed into a larger asynchronous operation.

A suggested change is to store the task returned by async method in a property so that the caller can use it to wait until load is completed, or compose it into an asynchronous method.

class DataLoader
{


public DataLoader ()
{
    //First load up current data from local sqlite db
    LoadFromLocal();

    //Then go for an async load from 
    this.Completion = AttemptDatabaseLoadAsync();
}

async Task AttemptDatabaseLoadAsync()
{
    while(ConnectionAttempts < MAX_CONNECTION_ATTEMPTS){
        Task<bool> Attempt = TryLoad ();
        bool success = await Attempt;
        if (success) {
            //call func to load data into program memory proper
        }else{
            ConnectionAttempts++;
        }
    }
}

public Task Completion
{
    get; private set;
}

}

Usage:

 var loader = new DataLoader();
 loader.Completion.Wait();

or:

async Task SomeMethodAsync()
{
   var loader = new DataLoader();
   await loader.Completion;
}