CODEWITHSUNDEEP
c#asp.netasynchronousasp.net-web-apiasync-await
Charly
Charly

Reputation: 851

Bad practice to return from method before async operation completes?

I have an Web API 2 end point where by I want to asynchronously carry out an operation while I retrieve and verify a user. If this user does not exist I want to return a 404 Not Found like so: 

public async Task<IHttpActionResult> Get()
{
    var getCatTask = GetCatAsync();
    var user = await GetUserAsync();

    if(user == null)
    {
        return NotFound();
    }     

    var cat = await getCatTask;

    return Ok(cat);
}

Could this cause me potential issues if the user was to equal to null and the method returned without awaiting the getCatTask or is it considered a bad practice?

Upvotes: 5

Views: 664

Answers (2)

David Pine
David Pine

Reputation: 24535

This is perfectly acceptable. In terms of async and await, you're not doing anything incorrect. A few notes about the keywords:

The async keyword simply enabled the usage of the await keyword. The await keyword is where all the "magic" happens, i.e.; the async state machine will suspend the method execution and return to that point when the "awaited" asynchronous operation has completed.

One important consideration:

Does the GetCatAsync() return a Task or Task<T> that represents an asynchronous operation that has already started? If so that might be a little problematic, if not you're fine then as it is awaited later. I would suggest adding cancellation though.

public async Task<IHttpActionResult> Get()
{
    var cts = new CancellationTokenSource();
    var getCatTask = GetCatAsync(cts.Token);
    var user = await GetUserAsync();

    if (user == null)
    {
        cts.Cancel();
        return NotFound();
    }     

    var cat = await getCatTask;
    return Ok(cat);
}

But, Stephen Cleary beat me to it -- called out above.

Upvotes: 3

Stephen Cleary
Stephen Cleary

Reputation: 457057

It's not really bad, since in this case you're only reading data and you're just going to ignore the result. You would incur the cost of an extra GetCatAsync operation for every fake request (which probably won't happen that often).

If possible, consider making GetCatAsync cancelable, and then you'll be able to at least start cleaning up before returning:

public async Task<IHttpActionResult> Get()
{
  var cts = new CancellationTokenSource();
  var getCatTask = GetCatAsync(cts.Token);
  var user = await GetUserAsync();

  if (user == null)
  {
    cts.Cancel();
    return NotFound();
  }     

  var cat = await getCatTask;
  return Ok(cat);
}

Upvotes: 9

Related Questions