175

I’m currently trying to make my application using some Async methods. All my IO is done through explicit implementations of an interface and I am a bit confused about how to make the operations async.

As I see things I have two options in the implementation:

interface IIO
{
    void DoOperation();
}

OPTION1: Do an implicit implementation async and await the result in the implicit implementation.

class IOImplementation : IIO
{

     async void DoOperation()
    {
        await Task.Factory.StartNew(() =>
            {
                //WRITING A FILE OR SOME SUCH THINGAMAGIG
            });
    }

    #region IIO Members

    void IIO.DoOperation()
    {
        DoOperation();
    }

    #endregion
}

OPTION2: Do the explicit implementation async and await the task from the implicit implementation.

class IOAsyncImplementation : IIO
{
    private Task DoOperationAsync()
    {
        return new Task(() =>
            {
                //DO ALL THE HEAVY LIFTING!!!
            });
    }

    #region IIOAsync Members

    async void IIO.DoOperation()
    {
        await DoOperationAsync();
    }

    #endregion
}

Are one of these implementations better than the other or is there another way to go that I am not thinking of?

Moriya
  • 7,750
  • 3
  • 35
  • 53

4 Answers4

312

Neither of these options is correct. You're trying to implement a synchronous interface asynchronously. Don't do that. The problem is that when DoOperation() returns, the operation won't be complete yet. Worse, if an exception happens during the operation (which is very common with IO operations), the user won't have a chance to deal with that exception.

What you need to do is to modify the interface, so that it is asynchronous:

interface IIO
{
    Task DoOperationAsync(); // note: no async here
}

class IOImplementation : IIO
{
    public async Task DoOperationAsync()
    {
        // perform the operation here
    }
}

This way, the user will see that the operation is async and they will be able to await it. This also pretty much forces the users of your code to switch to async, but that's unavoidable.

Also, I assume using StartNew() in your implementation is just an example, you shouldn't need that to implement asynchronous IO. (And new Task() is even worse, that won't even work, because you don't Start() the Task.)

svick
  • 236,525
  • 50
  • 385
  • 514
  • 1
    How would this look with an explicit implementation? Also, Where do you await this implementation? – Moriya Jan 22 '13 at 13:17
  • 2
    @Animal Explicit implementation would look the same as always (just add `async`): `async Task IIO.DoOperationAsync()`. And do you mean where do you `await` the returned `Task`? Wherever you call `DoOperationAsync()`. – svick Jan 22 '13 at 13:19
  • 1
    Basically I think I can condence my question into "Where do I await?" If I don't await inside the async method I get compilation warnings. – Moriya Jan 22 '13 at 13:21
  • Where do you await *what*? If you mean other asynchronous methods you're calling from inside `DoOperationAsync()`, then you certainly should `await` those. – svick Jan 22 '13 at 13:24
  • I don't have any other asyncronious calls to make inside the method but the compiler warning suggests I use await `TaskEx.Run(...)` inside the method to run CPU-bound work on a background thread. Will that be redundant if I await the method call in the first place i.e `await DoOperationAsync()` – Moriya Jan 22 '13 at 13:27
  • No, that won't be redundant. If you make any asynchronous calls in your method (which includes `Task.Run()`), then you should `await` them. – svick Jan 22 '13 at 13:31
  • Should I run then wrap the actual IO code in a `Task.Run()` and await it in the mthod as well as await the method when called from outside? – Moriya Jan 22 '13 at 13:55
  • 1
    Ideally, you shouldn't need to wrap the IO code in `Task.Run()`, that IO code should be asynchronous itself and you would `await` that directly. E.g. `line = await streamReader.ReadLineAsync()`. – svick Jan 22 '13 at 13:58
  • that is sadly not the case here. I'm doing a XmlSerialization and as far as I know there is not async implementation of such. – Moriya Jan 22 '13 at 14:00
  • 4
    Then there is not much point in making your code async. See the article [*Should I expose asynchronous wrappers for synchronous methods?*](http://blogs.msdn.com/b/pfxteam/archive/2012/03/24/10287244.aspx) – svick Jan 22 '13 at 14:05
31

Better solution is to introduce another interface for async operations. New interface must inherit from original interface.

Example:

interface IIO
{
    void DoOperation();
}

interface IIOAsync : IIO
{
    Task DoOperationAsync();
}


class ClsAsync : IIOAsync
{
    public void DoOperation()
    {
        DoOperationAsync().GetAwaiter().GetResult();
    }

    public async Task DoOperationAsync()
    {
        //just an async code demo
        await Task.Delay(1000);
    }
}


class Program
{
    static void Main(string[] args)
    {
        IIOAsync asAsync = new ClsAsync();
        IIO asSync = asAsync;

        Console.WriteLine(DateTime.Now.Second);

        asAsync.DoOperation();
        Console.WriteLine("After call to sync func using Async iface: {0}", 
            DateTime.Now.Second);

        asAsync.DoOperationAsync().GetAwaiter().GetResult();
        Console.WriteLine("After call to async func using Async iface: {0}", 
            DateTime.Now.Second);

        asSync.DoOperation();
        Console.WriteLine("After call to sync func using Sync iface: {0}", 
            DateTime.Now.Second);

        Console.ReadKey(true);
    }
}

P.S. Redesign your async operations so they return Task instead of void, unless you really must return void.

Dima
  • 1,717
  • 15
  • 34
  • 5
    Why not `GetAwaiter().GetResult()` instead of `Wait()`? That way you don't need to unpack an `AggregateException` to fetch the inner exception. – Tagc Oct 22 '17 at 22:58
  • 1
    A variation is rely on the class implementing multiple (possibly explicit) interfaces: `class Impl : IIO, IIOAsync`. IIO and IIOAsync themselves, however, are different contracts which can avoid pulling in 'old contracts' into newer code. `var c = new Impl(); IIOAsync asAsync = c; IIO asSync = c`. – user2864740 Jun 19 '19 at 23:27
  • Bad idea. If your operations are async, expose them as async, and let the consumer handle them as desired. The consumer can decide to "convert to sync" as your're doing in your example, or to keep being async. In fact, I'd recommend to also include always the Cancellation token, and use it, for example to time out the IO operations that are not done timely. Mosf of the async implementations in the .NET framework include it. – JotaBe Oct 07 '21 at 11:36
  • 2
    You shouldn't use the GetAwaiter and GetResult() in your code. It's meant for the compiler only: [TaskAwaiter Struct](https://learn.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.taskawaiter?view=net-5.0) – Roeland Van Heddegem Oct 27 '21 at 15:53
2

I created a sample app based on Svick's answer and found that calling IOImplementation.DoOperationAsync() without the async keyword does not result in a compiler/Visual Studio warning. This was based on Visual Studio 2019 and .NET Core 3.1.

Sample code below.

public interface ISomething
{
    Task DoSomethingAsync();
}
public class Something : ISomething
{
    public async Task DoSomethingAsync()
    {
        await Task.Run(() => Thread.Sleep(2000));
        Console.WriteLine("Message from DoSomethingAsync");

        throw new Exception("Some exception");
    }
}
class Program
{
    static void Main(string[] args)
    {
        ISomething something = new Something();
        
        Console.WriteLine("pre something.DoSomethingAsync() without await");
        something.DoSomethingAsync(); // No compiler warning for missing "await" and exception is "swallowed"
        Console.WriteLine("post something.DoSomethingAsync() without await");

        Thread.Sleep(3000);

        // Output:
        // pre something.DoSomethingAsync() without await
        // post something.DoSomethingAsync() without await
        // Message from DoSomethingAsync
    }
}
datchung
  • 3,778
  • 1
  • 28
  • 29
  • I believe that compiler have to warning about missing of "await" if your Main method is async. BTW, it's better to use Task.Delay instead of Thread.Sleep to implement waiting in async methods. – valker Feb 16 '22 at 20:10
-4

An abstract class can be used instead of an interface (in C# 7.3).

// Like interface
abstract class IIO
{
    public virtual async Task<string> DoOperation(string Name)
    {
        throw new NotImplementedException(); // throwing exception
        // return await Task.Run(() => { return ""; }); // or empty do
    }
}

// Implementation
class IOImplementation : IIO
{
    public override async Task<string> DoOperation(string Name)
    {
        return await await Task.Run(() =>
        {
            if(Name == "Spiderman")
                return "ok";
            return "cancel";
        }); 
    }
}
Alatey
  • 371
  • 2
  • 6
  • 1
    What is the benefit of doing this? Why does `IIO.DoOperation()` have a dummy implementation instead of being `abstract` itself? – Lance U. Matthews Aug 19 '20 at 00:10
  • In C # 7.3, I cannot use an "abstract async" method with no body. – Alatey Aug 19 '20 at 00:33
  • There is zero use in doing it this way. And there's no point in making the class `abstract` if there are no `abstract` members. The `DoOperation()` method could be made `abstract`, but then you've got something that is essentially just an interface. Which brings one back to, why not just use an interface? The code above is pointless. :( – Peter Duniho Aug 19 '20 at 00:35
  • 3
    _"I cannot use an "abstract async" method with no body"_ -- there is no reason to _want_ to use `abstract async`. The only thing that adding `async` to the method declaration does, is to allow you to use `await` in the method. If your method doesn't use `await`, then you don't need `async`. That's why the interface approach works in the first place; **what's actually important is the return type**. You can make the return type `Task` without the `async` keyword. – Peter Duniho Aug 19 '20 at 00:37