What is the best way to make multiple service calls with async await

Question

As a part of best practices for async and await it is recommended to not use Task.Run. I have a service which makes multiple calls to a third party service and we use async to make those calls. I'm looking for advice on code improvement in the code below.

  public interface IRouteService
{
     Task<IEnumerable<Route>> GetRoute(Coordinates orign, Coordinates destination);
}


public class RouteProvider
{
    private readonly IRouteService _routeService; 

    public RouteProvider(IRouteService routeService)
    {
        _routeService = routeService;
    }

    public async Task<IEnumerable<Route>> GetRoutes(IEnumerable<Coordinates> origns, IEnumerable<Coordinates> destinations)

    {
        ConcurrentBag<Route> routes = new ConcurrentBag<Route>();
        List<Task> tasks = new List<Task>();
        foreach (var origin in origns)
        {
            foreach (var destination in destinations)
            {
                tasks.Add(Task.Run(async () =>
                {
                  var response=  await _routeService.GetRoute(origin, destination);
                    foreach (var item in response)
                    {
                        routes.Add(item);
                    }
                }));
            }
        }
        Task.WaitAll(tasks.ToArray());
        return routes;
    }

}


public class Route
{
    public string Distance { get; set; }

    public Coordinates Origin { get; set; }

    public object Destination { get; set; }

    public string OriginName { get; set; }

    public string DestinationName { get; set; }

}

public class  Coordinates
{
    public float Lat { get; set; }

    public float Long { get; set; }


}

instead of `Task.WaitAll` you can write `return await Task.WhenAll(tasks);` — M.kazem Akhgary, Feb 03 '19 at 11:06

John Wu · Accepted Answer · 2019-02-04T06:07:19.510

For a problem like this it is handy to use LINQ. LINQ produces immutable results so you avoid concurrency issues and don't need any specialized collections.

In general, using LINQ or similar programming techniques (i.e. thinking like a functional programmer) will make multithreading much easier.

public async Task<IEnumerable<Route>> GetRoutes(IEnumerable<Coordinates> origins, IEnumerable<Coordinates> destinations)
{
    var tasks = origins
        .SelectMany
        (
            o => destinations.Select
            ( 
                d => _routeService.GetRoute(o, d) 
            )
        );
    await Task.WhenAll( tasks.ToArray() );
    return tasks.SelectMany( task => task.Result );
}

score 0 · Answer 2 · answered Feb 03 '19 at 11:58

As pointed in the comments I would suggest that you could use Task.WhenAll() to determine all task to complete and get the results with return await Task.WhenAll(tasks);. To do that, you can update your code like shown below.

public async Task<IEnumerable<Route>> GetRoutes(IEnumerable<Coordinates> origns, IEnumerable<Coordinates> destinations)
{
    ConcurrentBag<Route> routes = new ConcurrentBag<Route>();
    List<Task> tasks = new List<Task>();

    foreach (var origin in origns)
    {
        foreach (var destination in destinations)
        {
            tasks.Add(_routeService.GetRoute(origin, destination));
        }
    }

    var response = await Task.WhenAll(tasks);

    foreach (var item in response)
    {
        routes.Add(item);
    }
    return routes;
}

}

Since all the calls will return the same type, you do not need to start a second foreach in other loop. Also, this way you will avoid locking thread execution with Task.WaitAll() and your program will run more syncronous. To see the difference between WhenAll() vs WaitAll(), you can check this out.

score 0 · Answer 3 · answered Feb 03 '19 at 12:56

Instead of directly creating tasks using the Task.Run method you can use continuations.

foreach (var origin in origns)
{
    foreach (var destination in destinations)
    {
        tasks.Add(
            _routeService.GetRoute(origin, destination)
                .ContinueWith(response =>
                {
                    foreach (var item in response.Result)
                        routes.Add(item);
                })
        );
    }
}

Thus, the GetRoute method will be executed asynchronously, without creating a separate thread. And the result obtained from it will be processed in a separate thread (task).

However, this is only necessary if the result takes a long time to process. Otherwise, a separate thread is not needed at all.

What is the best way to make multiple service calls with async await

3 Answers3