1
\$\begingroup\$

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?

\$\endgroup\$
  • \$\begingroup\$ Do you actually need to wait on completion? \$\endgroup\$ – ratchet freak Nov 10 '15 at 14:28
  • 2
    \$\begingroup\$ 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 \$\endgroup\$ – IEatBagels Nov 10 '15 at 14:30
  • \$\begingroup\$ @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" \$\endgroup\$ – Jack Nov 10 '15 at 14:33
2
\$\begingroup\$

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.

\$\endgroup\$
2
\$\begingroup\$

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;
\$\endgroup\$
  • \$\begingroup\$ 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 ? \$\endgroup\$ – Jack Nov 11 '15 at 10:19
  • \$\begingroup\$ @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. \$\endgroup\$ – Gentian Kasa Nov 11 '15 at 10:31
  • 2
    \$\begingroup\$ @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. \$\endgroup\$ – svick Nov 11 '15 at 18:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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