10

I have two following methods

public async Task<bool> DoSomething(CancellationToken.token) 
{
    //do something async
}

//overload with None token 
public /*async*/ Task<bool> DoSomething()
{
    return /*await*/ DoSomething(CancellationToken.None);
}

Should second method be marked with async/await keywords or not?

svick
  • 236,525
  • 50
  • 385
  • 514
hazzik
  • 13,019
  • 9
  • 47
  • 86

2 Answers2

12

It doesn't need to - if you use await/async in the second method, you'll be adding extra overhead which accomplishes nothing, in this case.

The async work inside of DoSomething(CancellationToken) will already provide the proper asynchronous handling and marshaling back to the existing context for you.

At the end of the day, async and await are really just language features which make it easy to create and compose a Task. If you already have a perfectly good Task to return, there is no need to use extra language support to unwrap and rewrap it into a new Task.

Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • 1
    If I mark the method as async, and await before returning, then just before the return I can write some shortcircuit code like "if(someCondition) return false".. Without the async keyword this is not possible because it complains about return type mismatch... What would you recommend in this case? Use Task.FromResult?? Or is async await wrapping fine in this case? – labroo Nov 11 '14 at 06:35
  • 3
    @labroo I would, probably, use "Task.FromResult" in this case. – hazzik Feb 16 '15 at 20:34
12

To add to Reed's good answer, think about it this way:

Func<int, int> GetFunc()
{
    Func<int, int> f = GetFunc(someParameter);

Should you say

    return f;

or reason "I'm supposed to be returning a func here, so let's make a lambda that calls the func I have in hand":

    return (int i) => f(i);

I hope you would do the former; you already have a Func<int, int> in hand, so just return it. Don't make a func that calls a func.

If you had

IEnumerable<int> GetSequence()
{
    IEnumerable<int> sequence = GetSequence(someParameter);

would you say

    return sequence;

or

    foreach(int item in sequence) yield return item;

? Again, I hope you would do the former. You have a sequence in hand, so why go to all the trouble of making a new sequence that enumerates the old one?

The same goes for tasks; just like you can make a delegate that wraps another delegate and a sequence that wraps another sequence, you can make a task that wraps another task, but why would you? It's just a waste of resources.

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • 1
    Interestingly, ReSharper will detect (and offer to automatically correct) code which makes either of those two mistakes. I am not sure if it will detect the OP's case. – Brian Mar 18 '13 at 13:31
  • 4
    @Brian: Interesting. The latter case is arguably not a bug because it has an effect; first, it ensures that the resulting sequence does not have referential identity with the source sequence, and second, it ensures that the second sequence is *read only* even if the source sequence is not. – Eric Lippert Mar 18 '13 at 13:49