31

Assume I have an interface method implemented as

public void DoSomething(User user) 
{
    if (user.Gold > 1000) ChatManager.Send(user, "You are rich: " + user.Gold);
}

After some time I realize that I want to change it:

public async Task DoSomething(User user) 
{
    if (user.Gold > 1000) ChatManager.Send(user, "You are rich: " + user.Gold);
    if (!user.HasReward)
    {
         using(var dbConnection = await DbPool.OpenConnectionAsync())
         {
             await dbConnection.Update(user, u => 
                        {
                            u.HasReward = true;
                            u.Gold += 1000;
                        });
         }
    }
}

I'm changing the method signature in the interface. But the calling methods were synchronous and I have to make not only them async but also the whole call tree async.

Example:

void A()
{
    _peer.SendResponse("Ping: " + _x.B());
}

double X.B()
{
    return _someCollection.Where(item => y.C(item)).Average();
}


bool Y.C(int item)
{
   // ...
   _z.DoSomething();
   return _state.IsCorrect;
}

should be changed to

async void AAsync()
{
    _peer.SendResponse("Ping: " + await _x.BAsync());
}

async Task<double> X.BAsync()
{
    // await thing breaks LINQ!
    var els = new List<int>();
    foreach (var el in _someCollection)
    {
        if (await y.CAsync(item)) els.Add(el);
    }
    return _els.Average();
}


async Task<bool> Y.CAsync(int item)
{
   // ...
   await _z.DoSomething();
   return _state.IsCorrect;
}

The affected call tree may be very big (many systems and interfaces) so this change is hard to do.

Also when the first A method is called from interface method like IDisposable.Dispose - I can't make it async.

Another example: imagine that multiple calls to A were stored as delegates. Previously they were just called with _combinedDelegate.Invoke() but now I should go through GetInvocationList() and await on each item.

Oh, and consider also replacing property getter with async method.

I can't use Task.Wait() or .Result because:

  1. It's wasting ThreadPool threads in server app
  2. It leads to deadlocks: if all ThreadPool threads are Waiting there are no threads to complete any task.

So the question is: should I make absolutely all my methods initially async even if I'm not planning to call anything async inside? Won't it hurt performance? Or how to design things to avoid such hard refactorings?

6
  • You don't have to make the calling methods async - they can just do the call to the async method as var result = SomeAsyncMethod().Result. In this way, you can break the "async chain" until you are ready to refactor further up the call tree.
    – RB.
    Sep 4, 2016 at 21:04
  • Won't it hurt performance? Probably not in adverse ways. How to design things to avoid such hard refactorings? Asynchrony carries a price -- on one hand you can lazy-defer everything, but on the other, for non-trivial cases, it requires upstream code to support it (e.g. if your form validator engine doesn't support asynchrony, you're basically SOL and have to block on it). Sep 4, 2016 at 21:04
  • 2
    Infecting the call tree is the eternal torment of async codebases.
    – usr
    Sep 4, 2016 at 21:19
  • 3
    @Rb As a side note: You should always use GetAwaiter().GetResult() instead of .Result - gives you the right exception instead of one wrapped in an aggregate exception.
    – Voo
    Sep 5, 2016 at 6:26
  • @Voo I did not know that! The number of times I've written the same boilerplate code to unwrap exceptions is actually embarrassing now...! Thanks :-)
    – RB.
    Sep 5, 2016 at 7:01

2 Answers 2

26

should I make absolutely all my methods initially async even if I'm not planning to call anything async inside?

This design problem with async is essentially the same as the problem with IDisposable. That is, interfaces have to predict their implementations. And this is going to be messy, no matter what you do.

In my experience, it's usually rather straightforward to consider a method/interface/class and predict whether it will use I/O or not. If it requires I/O, then it should probably be made task-returning. Sometimes (but not always), it's possible to structure your code so that I/O is done in its own section of the code, leaving business objects and logic strictly synchronous. The Redux pattern in the JavaScript world is a good example of this.

But bottom line, sometimes you make the wrong call and have to refactor. I think this is a far better approach than just making every method asynchronous. Do you make every interface inherit from IDisposable and use using everywhere? Nah, you only add it when necessary; and you should take the same approach with async.

9
  • 2
    IDisposable is a much easier case: 1. often IDisposable objects are used only inside methods (not stored as a field anywhere) so only direct callers are affected. 2. Calling Dispose is often optional, you can just leave things for finalizer.; btw consider adding an async call to an existing IDisposable.Dispose method.
    – Vlad
    Sep 4, 2016 at 21:33
  • *I mean async call from Dispose
    – Vlad
    Sep 5, 2016 at 10:24
  • Are there any C# examples of "structure your code so that I/O is done in its own section of the code, leaving business objects and logic strictly synchronous"?
    – Vlad
    Sep 6, 2016 at 10:32
  • @Vlad: I'm not aware of anything specific. Sep 6, 2016 at 12:09
  • Not only IO requires await. Also switching between synchronization contexts requires await. I can't predict whether there will be interactions between users (each runs in its own sync context).
    – Vlad
    Sep 8, 2016 at 7:58
3

Should I make absolutely all my methods initially async even if I'm not planning to call anything async inside?

No, you should not. Making everything async hurts reading, writing and understanding your code, even if only a little. It could also hurt the performance of your code, especially if you really made every single method (does that include properties?) async.

How to design things to avoid such hard refactorings?

To some degree, such refactorings might be necessary.

But you can avoid a lot of the pain by structuring your code correctly using the single responsibility principle. A method that checks whether state is correct should definitely not send chat messages. This way, changing one method to async shouldn't affect too much code.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.