current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 1 down vote favorite

I've just started writing asynchronous methods for the first time having watched some tutorials.

I have a method where I run two tasks in parallel (notifySales, insertDownload):

Task insertDownload = _downloadService.InsertDownloadRequestAsync(model, Request.UserAgent);

if (product.NotifySalesByEmail)
{
    Task notifySales = _downloadService.EmailDownloadRequestAsync(model);
    await Task.WhenAll(insertDownload, notifySales);
}
else
{
    await insertDownload;
}

As you can see, the task notifySales only gets run if product.NotifySalesByEmail is true. This task, as well as task insertDownload can be run in parallel.

I'm not sure I like the way in which I have said 'if true await both else just await one'.

Would a better way to be to create a Task[] array, add the tasks that are required to run and then await the array?

share|improve this question
    
Do you actually need to wait on completion? – ratchet freak Nov 10 '15 at 14:28
2  
Can you add more context? At the moment it's hard to tell if that's the good way to proceed. Though it's not bad – TopinFrassi Nov 10 '15 at 14:30
    
@ratchetfreak In fact I don't. However, if I do not await the notifySales task then I get the following exception: "An asynchronous module or handler completed while an asynchronous operation was still pending" – JakkyD Nov 10 '15 at 14:33
up vote 2 down vote accepted

The code as it stands doesn't read well. And your text is also slightly confusing. I do believe I would rework the code to something along these lines:

  • Initialize an array of tasks
  • Add each task to array, with necessary conditions if needed
  • Await execution of all tasks

This would make for your code to handle extra tasks better, it would clear up the confusion on which tasks actually gets executed, and finally all tasks can be executed in parallell however the compiler feels like doing that.

share|improve this answer
up vote 2 down vote

Just taking into account the code that's present in the question here (and not considering a possible extension scenario in more tasks): Being that you have to wait for insertDownload anyway, I'd suggest to rewrite the code in the following way:

Task insertDownload = _downloadService.InsertDownloadRequestAsync(model, Request.UserAgent);

if (product.NotifySalesByEmail)
{
    await _downloadService.EmailDownloadRequestAsync(model);
}

await insertDownload;
share|improve this answer
    
First of all I would just like to say that I am very new to writing asynchronous code so please bear with me. InsertDownloadRequestAsync and EmailDownloadRequestAsync are two independent tasks that have no effect on each other, meaning they can be run in parallel. If I await EmailDownloadRequestAsync then does this mean InsertDownloadRequestAsync cannot start running? Or has InsertDownloadRequestAsync started running as soon as it has been assigned to Task insertDownload ? – JakkyD Nov 11 '15 at 10:19
    
@JakkyD maybe I sounded too inquisitive, sorry for that (I tend to appear that way when writing). That being said, in this particular case there's no big difference if you wait for one task to finish and then for the other. Seeing that you used await I guess the method is marked as async so this would be the best way IMO. In the general case of more tasks I'd suggest you to go with @holroy's approach. – Gentian Kasa Nov 11 '15 at 10:31
2  
@JakkyD await does not start Tasks, they start as soon as you call the method that returns it. So this answer will execute the two Tasks in parallel. – svick Nov 11 '15 at 18:21

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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