CODEWITHSUNDEEP
c#.netconventions
ewok
ewok

Reputation: 21443

C#: convention for synchronous method implementing an async interface

I have an interface that needs to be asynchronous. Most of the implementers of that interface will be asynchronous methods.

interface IValidator {
  Task<bool> ValidateAsync();
}

However, one implementation of it does not make any async calls, and I'm curious what the convention for implementing that particular method is. The way I see it, there are 2 options:

public async Task<bool> ValidateAsync() {
  return await Task.FromResult(SomeOtherMethod());
}

or

public Task<bool> ValidateAsync() {
  return Task.FromResult(SomeOtherMethod());
}

I'm curious what the proper convention is. Or is there some other option that I should use?

Upvotes: 0

Views: 91

Answers (1)

Stephen Cleary
Stephen Cleary

Reputation: 456407

To return a task, you can either create one yourself (Task.FromResult) or have the compiler create one for you (async).

Since there are no awaits, some people choose to create it themselves. However (as noted in the comments), this causes unexpected behavior with regards to exceptions. The implicit assumption for a task-returning method is that any exceptions from that method will be returned on that task, not thrown directly.

So, this code I would say is not ideal, since it raises exceptions directly to the caller:

public Task<bool> ValidateAsync() {
  // Any exceptions from SomeOtherMethod are propagated directly.
  return Task.FromResult(SomeOtherMethod());
}

A more correct build-your-own-task solution would look like this:

public Task<bool> ValidateAsync() {
  try {
    return Task.FromResult(SomeOtherMethod());
  }
  catch (Exception ex) {
    return Task.FromException<bool>(ex);
  }
}

And you may wish to make that more complex if you want to handle OperationCanceledException specially by returning a cancelled task (Task.FromCanceled). That step isn't usually necessary but some people prefer it (or need it, depending on how the rest of their code handles cancellation).

So, this pattern is already looking a bit complex, especially if you're repeating it.

The other option is to use async to create the task for you. And, while you can do this with await Task.FromResult, that's really just adding overhead compared to this:

public async Task<bool> ValidateAsync() {
  return SomeOtherMethod();
}

This solution is good in that it properly handles exceptions/cancellation (i.e., all control flows). This solution has a problem, though: it causes a CS1998 compiler warning. To be clear, the compiler warning is a good thing, because a missing await is usually a mistake, and the warning is very clear about how the method will run synchronously. Which in this case is what we want.

So, my preferred solution is to just do the above with the compiler warning disabled. If this is a pattern you plan to reuse several times in your code, you can add a trivial helper method like this so that the #pragma only happens in one place:

public static class TaskHelper
{
#pragma warning disable 1998
  public static async Task ExecuteAsTask(Action func)
#pragma warning restore 1998
  {
    _ = func ?? throw new ArgumentNullException(nameof(func));
    func();
  }

#pragma warning disable 1998
  public static async Task<T> ExecuteAsTask<T>(Func<T> func)
#pragma warning restore 1998
  {
    _ = func ?? throw new ArgumentNullException(nameof(func));
    return func();
  }
}

Usage:

public Task<bool> ValidateAsync() {
  return TaskHelpers.ExecuteAsTask(() => SomeOtherMethod());
}

Upvotes: 1

Related Questions