Async and sync versions of method [duplicate]

Question

Ok so I have been reading up a lot and working on the best ways to use async methods and task, etc. I believe I'm (mostly) understanding it but I want to check to make sure.

I did start making async wrappers for sync tasks using Task.Run() but recently read that it's much better to just have a sync method and let the application determine if it needs to be pushed to another thread by using Task.Run() itself, which makes sense. However, I've also read that the exception to this is for naturally async functions. I'm not sure I fully understand 'naturally async' but as a basic understanding it seems that .NET framework methods that offer async methods are naturally async and WebClient.DownloadFile/DownlodFileAsync is one of those.

So I have two methods and if anyone would be willing to give feedback I would like to test my understanding and make sure this is correct.

Method one is for moving some files around on the local operating system and looks like this (pseudo-code):

Public static void MoveStagingToCache()
{
    ...do some file move, copy, delete operations...
}

The second looks like this (pseudo-code):

Public static void DownloadToCache()
{
    ...do some analysis to get download locations...
    using (var wc = new WebClient())
    {
        wc.DownloadFile(new Uri(content.Url), targetPath);
    }
    ...do other stuff...
}

So my understanding is as follows. The first method should be left as is, since none of the file operation methods have async versions and are therefore not likely naturally async. It would be up to the caller to determine between just calling MoveStagingToCache() to run sync or calling Task.Run(()=>MoveStagingToCache()) to push it to a background thread.

However, in the second method, the download is naturally async (unless I misunderstand) so it could create a sync and async version. In order to do that I should NOT just wrap the sync method like this:

Public static Task DownloadToCacheAsync()
{
    return Task.Run(()=>DownloadToCache());
}

Instead, I should make the core method async as follows:

Public static async Task DownloadToCache()
{
    ...do some analysis to get download locations...
    using (var wc = new WebClient())
    {
        await wc.DownloadFileTaskAsync(new Uri(content.Url), targetPath);
    }
    ...do other stuff...
}

and then I can create the sync version like this:

Public static void DownloadToCache()
{
    DownloadToCacheAsync().Wait();
}

This allows for use of the naturally async method and also offers a sync overload for those that need it.

Is that a good understanding of the system or am I messing something up?

Also as an aside, is the difference in WebClient.DownloadFileAsync and WebClient.DownloadFileTaskAsync just that the task returns a task for error catching?

EDIT

Ok so after some discussion in the comments and answers I realized I should add some more detail on my system and intended use. This is inside a library that is intended to be run on desktop, not ASP. As such, I'm not concerned about saving threads for request processing, the main concern is to keep the UI thread open and responsive to the user and to push 'background' tasks to another thread that can be processed by the system while the user goes about doing what they need to do.

For MoveStagingToCache, this will be called on program startup, but I don't need to wait for it to complete to continue loading or using the program. In most cases it will complete before the rest of the program is up and running and lets the user do anything most likely (probably 5-10 seconds run time max), but even if it's not complete before user interaction starts the program will work fine. Because of that my main desire here is to move this operation off the UI thread, start it working, and continue on with the program.

So for this one, my current understanding is that this method should be sync in the library, but then when I call it from the main (UI) thread I just use

Task.Run(()=>MoveStagingToCache());

Since I don't really need to do anything when complete, I really don't need to await this right? If I just do the above it will start the operation on a background thread?

For DownloadToCache, similar but a little different. I want the user to be able to initiate the download operation, then continue working in the UI until the download is complete. After it's complete I will need to do some simple operations to notify the user that it's ready to go and enable the 'use it' button, etc. In this case, my understanding is that I would create this as an async method that awaits the WebClient download call. This would push it off the UI thread for the actual download, but then would return once download is complete to allow me to do whatever UI updates are needed after the await call.

Correct?

Answer 1

You shouldn't write asynchronous wrappers for synchronous methods, but you also shouldn't write synchronous wrappers for asynchronous methods - these are both antipatterns.

Tip: "naturally asynchronous" mostly means I/O-based, with a few exceptions. One of those exceptions is some filesystem operations, unfortunately, including moving files, which should be asynchronous, but the APIs don't support asynchrony, so we have to pretend it's synchronous.

In your case, DownloadToCache is definitely naturally asynchronous. In those cases, I prefer exposing an asynchronous API only.

If you must (or really want to) support a synchronous API as well, I recommend the boolean argument hack. The semantic is that if you pass in sync:true, then the returned task is already completed. This allows you to keep your logic in a single method and write very small wrappers without the pitfalls normally associated with those wrappers:

private static async Task DownloadToCacheAsync(bool sync)
{
  ...do some analysis to get download locations...
  using (var wc = new WebClient())
  {
    if (sync)
      wc.DownloadFile(new Uri(content.Url), targetPath);
    else
      await wc.DownloadFileTaskAsync(new Uri(content.Url), targetPath);
  }
  ...do other stuff...
}
public static Task DownloadToCacheAsync() => DownloadToCacheAsync(sync: false);
public static void DownloadToCache() => DownloadToCacheAsync(sync: true).GetAwaiter().GetResult();