Is this usage of await/async correct?

Question

I'm new to async/await and I want to make sure that this way of doing it is correct:

public async Task DoHeavyWorkAsync() 
{
    await Task.Run(() => {
        getResponseFromFarawaySlowServerAndDoSomethingWithIt();
    });
}

public async void ConsumeAsync()
{
    Task longRunningTask = DoHeavyWorkAsync();
    // do a lot of other stuffs here that does not depend on DoHeavyWorkAsync()
    await longRunningTask;
}

Is this way of using async/await correct or did I do something wrong?

Answer 1

There are a couple of things you can do:

1. In DoHeavyWorkAsync, you don't really need to generate a state machine using await Task.Run, you can simply return Task.Run:

   public Task DoHeavyWorkAsync() 
   {
      return Task.Run(() => getResponseFromFarawaySlowServerAndDoSomethingWithIt());
   }
   

2. async void is ment solely for async Event Handlers. If your async method is void returning, it should return a Task instead:

   public async Task ConsumeAsync()
   

3. If DoHeavyWorkAsync is an IO based operation, there is no need to wrap it inside a Task.Run as it is inherently asynchronous. Simply using await will do. More-so, you shouldn't do async over sync. Instead, you should make the caller of the synchronous method explicitly use Task.Run, if needed at all:

   public void DoHeavyWork()
   {
       getResponseFromFarawaySlowServerAndDoSomethingWithIt();  
   }
   

   and then explicitly wrap it in the calling method:

   Task.Run(DoHeavyWork);
   

Answer 2

from API designer point of view, I would consider spliting method getResponseFromFarawaySlowServerAndDoSomethingWithIt to:
getResponseFromFarawaySlowServer and doSomething().
Then you can wrap only the long running method with async wrapper

usage would then:

var response = await getResponseFromFarawaySlowServerAsync();
doSomething(response);

another thing that smells a little bit: getResponseFromFarawaySlowServer itself is not async. the http call or webservice call itself should be awaited inside that method if possible. Currently you are creating new thread thad does nothing just wait. This is be redundant if you awaited the http call instead

so instead

string getResponseFromFarawaySlowServer(){
  string response = new WebClient().DownloadString(uri);
  ...
  return response
}

async Task<string> getResponseFromFarawaySlowServerAsync(){ Task.StartNew..

you would directly:

async Task<string> getResponseFromFarawaySlowServerAsync(){
  string response = await new WebClient().DownloadStringAsync(uri);
  ...
  return response;
}