92

What would be the best way to convert/wrap a "classic" asynchronous method that uses a callback to something that returns a (awaitable) Task?

For example, given the following method:

public void GetStringFromUrl(string url, Action<string> onCompleted);

The only way I know of to wrap this into a method returning a task is:

public Task<string> GetStringFromUrl(string url)
{
     var t = new TaskCompletionSource<string>();

     GetStringFromUrl(url, s => t.TrySetResult(s));

     return t.Task;
}

Is this the only way to accomplish this?

And is there a way to wrap the call to GetStringFromUrl(url,callback) in the task itself (i.e. the call itself would run inside the task instead of synchronously)

Philippe Leybaert
  • 168,566
  • 31
  • 210
  • 223
  • 2
    BTW, this is not “classic” .net asynchronous method. Those are `BeginXxx()` and `EndXxx()` pairs. Also, why are you looking for other ways to do this? What are you hoping to gain? – svick Aug 09 '12 at 12:10
  • 9
    I just want to make sure I'm not missing some obvious alternative way of doing the same thing. – Philippe Leybaert Aug 09 '12 at 14:23
  • 1
    is there any reason to use `TrySetResult` instead of `SetResult` here? – Ben Huber Feb 11 '20 at 21:58

2 Answers2

51

Your code is short, readable and efficient, so I don't understand why are you looking for alternatives, but I can't think of anything. I think your approach is reasonable.

I'm also not sure why do you think that the synchronous part is okay in the original version, but you want to avoid it in the Task-based one. If you think the synchronous part might take too long, fix it for both versions of the method.

But if you want to run it asynchronously (i.e. on the ThreadPool) only in the Task version, you can use Task.Run():

public Task<string> GetStringFromUrl(string url)
{
    return Task.Run(() =>
    {
        var t = new TaskCompletionSource<string>();

        GetStringFromUrl(url, s => t.TrySetResult(s));

        return t.Task;
    });
}
svick
  • 236,525
  • 50
  • 385
  • 514
  • 2
    I don't always control the existing callback-based method, so if the synchronous part of the method is doing something that takes too long, I'd like to have to option to move that in a task as well. Your solution solves just that. – Philippe Leybaert Aug 09 '12 at 14:25
  • 1
    Hmmm... I was assuming the original GetStringFromUrl with the callback is already async (hence the callback). If so, this suggestion is going to burn resources spinning off another task just to invoke the call. – Drew Marsh Aug 09 '12 at 22:18
  • @DrewMarsh Even async methods usually have some synchronous part, although most of the time it's negligible. But the question specifically asked for that, I wouldn't suggest anything like that otherwise. – svick Aug 09 '12 at 23:09
  • 1
    @svick Sure, there's usually some startup cost, but unless someone created a really shoddy implementation I would be dimes to dollars that you'd be spending more overhead throwing it over to another thread with Task.Run than just having that part execute on the current thread. I think you kinda gotta trust the async API you're calling in that regard until you can prove it guilty and I personally would never recommend doing it by default. Just my 2¢. – Drew Marsh Aug 10 '12 at 03:11
  • If this method is really async then starting a new task will lose all the advantages of asynchronous running jobs. If you want you can just call Task.Yield() before method call to post it into the task scheduler. Question's code will be better in any way. – Alex Lyalka Aug 09 '19 at 11:46
  • @AlexLyalka can you share a code sample of what you mean? I'm trying to understand what the difference is between the OPs code, this code, and what you described – user3010678 Dec 11 '22 at 15:59
6

Your assumed implementation is perfectly fine for this assuming the callback only ever handles successful situations. What currently happens if an exception happens within the async underpinnings of the GetStringFromUrl implementation? There's no real way for them to propagate that to the Action callback... do they just swallow it and return you null or something?

The only thing I would recommend is using following the new convention of naming such async methods with the XXXAsync suffix.

Drew Marsh
  • 33,111
  • 3
  • 82
  • 100