0
\$\begingroup\$

In my project I have an async method AnAsynWebServiceCallHere for calling a web service. I want to call AnAsynWebServiceCallHere twice in parallel and at the end I want to return the combined result. Will at the end of this method

public async Task<List<DesiredResult>> GetMyDesiredData(MyParamDTO dto)
{
  List<DesiredResult> list = new List<DesiredResult>();
  await Task.WhenAll(
    Task.Run(()=> {var result1 = AnAsynWebServiceCallHere(dto.A);list.Add(result1);}), 
    Task.Run(()=> {var result2 = AnAsynWebServiceCallHere(dto.A);list.Add(result2);})
  );
  return list;
}

and body of the '' method is:

public async Task<DesiredResult> AnAsynWebServiceCallHere(string sqlQuery)
{
  string json;
  using(HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Post, "https://demoapi.MyHost.net/SQLRunner"))
  {
      request.Headers.Add("id", SECURITY_TOKEN);          
      request.Headers.Add("sqlStatement", sqlQuery);
      HttpResponseMessage response = await client.SendAsync(request);
      json = await response.Content.ReadAsStringAsync();
  }
  return JsonConvert.DeserializeObject<DesiredResult>(json);
}

Is the written method an elegant approach?

Update: Context provided with the called method body.

\$\endgroup\$
4
  • \$\begingroup\$ We cannot tell you whether this is an elegant approach or not becase this code isn't real and we don't review pseudo/hypothetical code. \$\endgroup\$
    – t3chb0t
    Commented Sep 28, 2018 at 5:37
  • \$\begingroup\$ This is a real code, just changed the method and object names from my real project. is the down-vote due to this reason? \$\endgroup\$ Commented Sep 28, 2018 at 6:44
  • \$\begingroup\$ Yes, the DV is for that reason and for not sharing enough context e.g. we don't know what's the signature of AnAsynWebServiceCallHere. \$\endgroup\$
    – t3chb0t
    Commented Sep 28, 2018 at 6:46
  • \$\begingroup\$ I find this still looks very much like pseudocode and thus voted against reopening but some people are of a different opinion so it might get reopened soon... \$\endgroup\$
    – t3chb0t
    Commented Sep 29, 2018 at 7:25

1 Answer 1

2
\$\begingroup\$

If AnAsynWebServiceCallHere is already async then there is no need for the additional Task.Run in the Task.WhenAll.

Task.WhenAll<TResult>(param Task<TResult>[]) will already return the results of the tasks in the collection as an array of results.

All that is left then is to convert the array to a List<>

public async Task<List<DesiredResult>> GetMyDesiredData(MyParamDTO dto) {
    var results = await Task.WhenAll(
        AnAsynWebServiceCallHere(dto.A), 
        AnAsynWebServiceCallHere(dto.A)
    );
    return results.ToList();
}

All the tasks will run in parallel and at the end will return the combined results.

Looking at the AnAsynWebServiceCallHere you could also wrap the response in a using statement as it is disposable also

public async Task<DesiredResult> AnAsynWebServiceCallHere(string sqlQuery) {
    var url = "https://demoapi.MyHost.net/SQLRunner";
    using(var request = new HttpRequestMessage(HttpMethod.Post, url)) {
        request.Headers.Add("id", SECURITY_TOKEN);          
        request.Headers.Add("sqlStatement", sqlQuery);
        using(HttpResponseMessage response = await client.SendAsync(request)) {
            var json = await response.Content.ReadAsStringAsync();
            return JsonConvert.DeserializeObject<DesiredResult>(json);
        }
    }      
}
\$\endgroup\$

Your Answer

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

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