I think this is one of those things that doesn't concretely matter enough to die on either hill; but developers tend to err one way or another. I, for one, err the other way than you. This answer is my reasoning, but at the end of the day both our approaches yield working solutions, there is no right and wrong here.
Taking a bit of a detour, let's refresh our definition of encapsulation. We abstract away the concrete, in order for the consumer to no longer be attached to the concrete implementation but rather the contract.
The idea here is that by doing so, we are about to swap out our implementation without impacting the consumer, as long as we can adhere to the contract that the consumer was built to work with.
Again a small detour, I want to focus on methods in general for a second, not just API endpoints. I reckon the spirit of encapsulation is best continued by making all signatures async, because then the concrete implementation is free to choose whether it implements a synchronous or an asynchronous solution.
In a sense, having a synchronous signature reveals to the consumer that you are working synchronously. If you ever need to work asynchronously, you need to disrupt the established contract. Vice versa, this would not have been a problem, as you could have made a previously async method synchronous without needing to disrupt the contract.
Therefore, erring towards async signatures is more change-friendly than only using them when the concrete need arises, which is why I'm in favor of sticking with a consistent async signature once my application is predominantly async.
Focusing back to API endpoints, the argument is slightly weaker, since the framework handles async/sync endpoints for you. There is no real consumer that you need to take care of since the consumer is .NET and it can adapt to either.
Maybe you consider that a reason to ignore the above reasoning. I could see how you get there. But I am a stickler for consistency in a codebase, and since the same reasoning applies, even if only on principle, I prefer to keep things consistent instead of introducing exceptional sync cases here and there.
Are there exception to this? Of course. Not every extension method or helper class I write is therefore async, because there are some cases where the remit of the class is clearly not of an asynchronous nature. This is not a dogma nor a blanket statement, and I'm not trying to sell it as such.
Update To be clear, because I notice from the comments that this was left too ambiguous, I am not suggesting you use the async
keyword everywhere. My focus here is on the contracts being async-oriented.
The async
keyword doesn't impact the contract (you can freely change it without forcing the consumer to make any change), but things like adding/removing the Task<T>
return type, passing a CancellationToken
, ... does impact the contract, and it's those things that I would add even if you know that the method body is currently synchronous.
In other words, I would leave the method differently than either of your examples. I would be more inclined to do:
[HttpGet]
public Task<IActionResult> Foo()
{
IActionResult result = OK(...);
return Task.FromResult(result);
}
No async
keyword since there is no await
happening. However, I did keep the Task<T>
return type as this forms part of the contract.
As an aside, I think part of this is due to async only having appeared on the stage much later than when synchronous execution did.
Because of it, using an async signature feels like doing something extra on top of the (synchronous) baseline, which in turn triggers developers' need to be efficient and makes them go "why would I do this extra thing when I don't need it right now? YAGNI!".
I think that's the wrong way around. I think our baseline should be the most permissive options, in this case async (which, as established), can also house sync operations). We should only strengthen the condition to only allow async when there are cases where we either clearly don't need or explicitly want to disallow asynchronous execution.
Had we have had async right from the get go with sync, I reckon we wouldn't have thought about async as a quirky extra. But we didn't, and so we don't use async as the baseline since we were first taught synchronous operations. Maybe the next generation of developers will be different now that async is here to stay.
Then one of my teammates claims controller actions should always be async
did he or she give you any argument?async
keyword by itself (without anawait
) does no such thing, it merely enables theawait
keyword, nothing more - certainly nothing to do with threads (while it changes the method return type so that the result is wrapped in aTask
,Task
by itself does not alter behaviour). ifawait
isn't used, then it is merely a plain synchronous method. Stephen Cleary's blog has a good, in-depth explanation: blog.stephencleary.com/2012/02/async-and-await.html