.Net Async ContinueWith VS Embedding Tasks in Task

Question

Just wondering the best approach when it comes to async. At first my code looked like this (example is simplified).

public NotificationSummary SendNotification()
{
      var response = new NotificationSummary();
      var first = FindSubscriptions(1);
      ... 
      var seventh = FindSubscriptions(7);

      Task.WaitAll(first, ... , seventh);

      response.First = first.Result;
      ...
      response.Seventh = seventh.Result;
      return response;
}


private Task<NotificationResult> FindSubscriptions(int day)
{
     return Task.Run(() => 
     {
        var subscriptions = // call to database to get list of subscriptions
        var tasks = subscriptions.Select(x =>  SendOutNotification(x))
        var results = Task.WhenAll(tasks).Result.ToList();
        return // map results to NotificationResult
     }
}


private Task<IndividualResult> SendOutNotification(Subscription subscription)
{
    return Task.Run(() => 
    {
       var response = new IndividualResult();
       foreach(var user in subscription.Users)
       {
            try
            {
                 // Send user info to EMAIL API
                 response.Worked.Add(user);
            }
            catch(Exception ex) { response.Failed.Add(user)}
       }

       return response;
    }
}

But this approach is violating single responsibility and it might be confusing to other developers when they come try figure out what this code is doing. I was trying to find a way to chain together the tasks and I came across ContinueWith. I have done some research (aka looked at other stackoverflow posts) and I get mixed reviews on ContinueWith. I would really like my SendNotification method to look like this, but I don't know if this is a good approach when it comes to async and tasking.

public NotificationSummary SendNotification()
{
    var response = new NotificationSummary();
    var firstTasks = new List<IndivdualResult>();
    var first = FindSubscriptions(1).ContinueWith( x=>
                  x.Result.ForEach(r =>
                    firstTasks.Add(SendOutNotification(x).Result)));
   response.First = // map first;

   // do 2 - 7 tasks as well

   return response;
}

private Task<List<Subscription>> FindSubscriptions() {} //returns subscriptions

private Task<IndividualResults> SendOutNotication() {} // same as above

I am wondering which one of these approaches would be considered the "right way" if either?

Answer 1

ContinueWith is a code smell since await became available. await is basically a nice way of attaching a continuation.

I see no structural problem with the first version of your code. You probably should:

• replace all Wait/Result calls with await
• delete Task.Run usages
• replace WaitAll with await WhenAll
• replace ContinueWith with a new async method and await

This should clean up the mess and fix the efficiency problems.

If you have no need for parallelism you also can make everything synchronous.