Refactoring a library to be async, how can I avoid repeating myself?

Question

I have a method like so:

    public void Encrypt(IFile file)
    {
        if (file == null)
            throw new ArgumentNullException(nameof(file));

        string tempFilename = GetFilename(file);
        using (var stream = new FileStream(tempFilename, FileMode.OpenOrCreate))
        {
            this.EncryptToStream(file, stream);
            file.Write(stream);
        }

        File.Delete(tempFilename);
    }

However, I'm wanting to write another method, very similar but it calls WriteAsync instead, such as:

    public async Task EncryptAsync(IFile file)
    {
        if (file == null)
            throw new ArgumentNullException(nameof(file));

        string tempFilename = GetFilename(file);
        using (var stream = new FileStream(tempFilename, FileMode.OpenOrCreate))
        {
            this.EncryptToStream(file, stream);
            await file.WriteAsync(stream);
        }

        File.Delete(tempFilename);
    }

However I don't like having two methods with practically duplicate code. How can I avoid this? The right approach feels like I should use an Action/Delegate, but the signatures are different....

Thoughts?

Answer 1

However I don't like having two methods with practically duplicate code. How can I avoid this?

There are a few approaches, as outlined in my MSDN article on brownfield async development.

1) Make naturally-asynchronous APIs asynchronous-only.

This is the most drastic solution (in terms of backwards compatibility), but from a technical perspective, you could argue it's the most correct. With this approach, you would replace Encrypt with EncryptAsync.

While this is the best approach IMO, your end users may not agree. :)

2) Apply the hack of blocking on the asynchronous version.

public void Encrypt(IFile file)
{
  EncryptAsync(file).GetAwaiter().GetResult();
}

Note that (as I describe on my blog), to avoid deadlocks you'll need to use ConfigureAwait(false) everywhere in your asynchronous version.

The disadvantages to this hack:

• You literally have to use ConfigureAwait(false) for every await in your async version and every method it calls. Forget even one and you've got a deadlock possibility. Note that some libraries do not use ConfigureAwait(false) everywhere for all platforms (notably, HttpClient on mobile platforms).

3) Apply the hack of running the asynchronous version on a thread pool thread and blocking on that.

public void Encrypt(IFile file)
{
  Task.Run(() => EncryptAsync(file)).GetAwaiter().GetResult();
}

This approach avoids the deadlock situation entirely, but has its own disadvantages:

• The asynchronous code has to be able to be run on a thread pool thread.
• The asynchronous code may be run on multiple thread pool threads throughout its existence. If the asynchronous code implicitly depends on the synchronization of a single-threaded context, or if it uses thread-local state, then this approach won't work without some rewriting.

4) Pass in a flag argument.

This is the approach I like the best, if you're unwilling to take approach (1).

private async Task EncryptAsync(IFile file, bool sync)
{
  if (file == null)
    throw new ArgumentNullException(nameof(file));

  string tempFilename = GetFilename(file);
  using (var stream = new FileStream(tempFilename, FileMode.OpenOrCreate))
  {
    this.EncryptToStream(file, stream);
    if (sync)
      file.Write(stream);
    else
      await file.WriteAsync(stream);
  }

  File.Delete(tempFilename);
}

public void Encrypt(IFile file)
{
  EncryptAsync(file, sync: true).GetAwaiter().GetResult();
}

public Task EncryptAsync(IFile file)
{
  return EncryptAsync(file, sync: false);
}

The boolean flag is certainly ugly - and a red flag to proper OOP design - but it's hidden in a private method, and not as dangerous as the other hacks.

There's a couple other hacks covered in my article (single-threaded thread pool context, and nested message loops), but I generally don't recommend them.

---

On a side note, if your code really is using FileStream, you need to explicitly open it for asynchronous access to get true asynchronous operations. That is, you have to call the constructor either passing true for the isAsync parameter, or setting the FileOptions.Asynchronous flag in the value for the options parameter.