Reputation: 2961
I have one service lets say,
public interface ISomeService
{
Task<bool> DoSomeExpensiveCheckAsync(string parameter);
}
And I have this class to consume the service. It just needs to do some simple null checks and then return the service response back.
public class SomeServiceConsumer
{
private readonly ISomeService _serviceClient;
public SomeServiceConsumer(ISomeService serviceClient)
{
_serviceClient = serviceClient;
}
public async Task<bool> DoSomething1Async(string someParameter)
{
if (string.IsNullOrWhiteSpace(someParameter))
{
return false;
}
return await _serviceClient.DoSomeExpensiveCheckAsync(someParameter);
}
//No async or await keywords
public Task<bool> DoSomething2Async(string someParameter)
{
if (string.IsNullOrWhiteSpace(someParameter))
{
return Task.FromResult(false);
}
return _serviceClient.DoSomeExpensiveCheckAsync(someParameter);
}
}
Should I do DoSomething1Async
or DoSomething2Async
?
According to this answer, I should not wrap with an unnecessary await
but then I have to use Task.FromResult(false)
for shortcircuiting as in DoSomething2Async
But according to this answer there are cases with try/catch
and using
statements where I actually should await
before returning.
Am I correct in saying then, that
If I have to use try/catch
or using
then I should await
Otherwise do not await
if you are only going to return. And use Task.FromResult
for short circuiting
I like DoSomething1Async
more, and want to do that everywhere if someone says it doesnt matter :).
Upvotes: 23
Views: 20252
Reputation: 27049
To answer your own question, you need to ask yourself this question: Which part of the method is the truly async
part? I think we can all agree that the truly async
part is when the code _serviceClient.DoSomeExpensiveCheckAsync
is called. Therefore, DoSomething2Async
is more a hack than anything. According to MSDN:
This method is useful when you perform an asynchronous operation that returns a Task object, and the result of that Task object is already computed.
So in other words if you had computed the result of the truly asynchronous part or you had it cached, you could use Task.FromResult
on it. However, using Task.FromResult(false)
would be lying and hacking the whole mechanism.
In some cases you may need to use Task.FromResult
for doing work that's not truly asynchronous but the work may take a while because it is CPU intensive, in those cases one may make an exception to avoid freezing the UI.
In conclusion, DoSomething1Async
is more appropriate.
Upvotes: -1
Reputation: 30021
If you're worried about it, cache the Task
:
static readonly Task<bool> falseTask = Task.FromResult(false);
The async
keyword also wraps exceptions up in the returned Task
, along with a proper stack trace. It's a tradeoff, safety of behavior for perf.
Lets look at the difference scenarios where each would be different:
async Task UseSomething1Async(string someParameter)
{
// if IsNullOrWhiteSpace throws an exception, it will be wrapped in
// the task and not thrown here.
Task t1 = DoSomething1Async(someParameter);
// rather, it'll get thrown here. this is best practice,
// it's what users of Task-returning methods expect.
await t1;
// if IsNullOrWhiteSpace throws an exception, it will
// be thrown here. users will not expect this.
Task t2 = DoSomething2Async(someParameter);
// this would never have been reached.
await t2;
}
Just illustrating the point here -- IsNullOrWhiteSpace
does not actually throw any exceptions for any reason.
As far as stack traces go, async stack traces are determined by where you await
. No await
means the method will disappear from the stack trace.
Say DoSomeExpensiveCheckAsync
throws an exception. In the case of DoSomething1Async
, the stack trace will look like caller -> DoSomething1Async -> DoSomeExpensiveCheckAsync
.
In the case of DoSomething2Async
, the stack trace would look like caller -> DoSomeExpensiveCheckAsync
. Depending on the complexity of your code, this can make things difficult to debug.
In practice, I will generally only directly return a Task
if I knew no exceptions would be thrown prior to it, and if the method name was merely an overload forwarding to another overload. There are always exceptions to this rule, there are bound to be places you want to maximize performance. Just pick and choose carefully, realize you might be making the life of you and your user harder.
Upvotes: 18
Reputation: 116666
It doesn't really matter. If you're comfortable with always marking Task
-returning methods with the async
keyword then go ahead and use DoSomething1
.
As you said, it's a tradeoff:
DoSomething2
doesn't generate the state machine needed for an async
method and so it's slightly faster (but the difference is mostly negligible).
On the other hand it can have some unforeseen side effects regarding exception handling since in an async
method the exception would be stored in the returned Task
and in the other it would be thrown regularly.
Upvotes: 4