1
\$\begingroup\$

This service takes in a report request object that can contain multiple reports that need to be rendered. This code works but I'm not sure if I've implemented anything wrong. Note the exportedImage.Result makes this synchronous but when I tried adding async and await to the Parallel.ForEach it doesn't seem to work so this was essentially a workaround, that's why I'm looking to figure out if I've implemented this correctly or not. _exporter.Export is an asynchronous method.

Just FYI, when I tried to add async and await, I added it like so:

await Task.Run(() => Parallel.ForEach(request.Reports, async report =>

var exportedImage = await _exporter.Export(_reportServerUrl, reportPath, export, reportParameters);

var reportDictionary = new ConcurrentDictionary<int, string>();

await Task.Run(() => Parallel.ForEach(request.Reports, report =>
{
    var (reportPath, bindingOrder, reportParameters) = report;
    
    var exportedImage = _exporter.Export(_reportServerUrl, reportPath, export, reportParameters);

    if (exportedImage.Result == null)
    {
        throw new InvalidOperationException($"Could not render report {reportIdentifier} with binding order {bindingOrder}.");
    }

    reportDictionary.GetOrAdd(bindingOrder, exportedImage.Result);
}));
\$\endgroup\$
3
  • 2
    \$\begingroup\$ No, you're not using it properly. Do not mix async/await with the TPL methods (Parallel.). They're two totally different approaches to scalability. \$\endgroup\$ Commented Mar 17, 2021 at 17:49
  • \$\begingroup\$ @JesseC.Slicer for my scenario, would you recommend going TPL or only async await? \$\endgroup\$
    – Usman Khan
    Commented Mar 18, 2021 at 11:18
  • 1
    \$\begingroup\$ I rather like the answer that was accepted. Modern, clean. \$\endgroup\$ Commented Mar 18, 2021 at 12:26

1 Answer 1

2
\$\begingroup\$

Here's pure async example without any workarounds.

private async Task ProcessReport(Report report, ConcurrentDictionary<int, string> reportDictionary)
{
    var (reportPath, bindingOrder, reportParameters) = report;
    
    var exportedImage = await _exporter.Export(_reportServerUrl, reportPath, export, reportParameters);

    if (exportedImage == null)
    {
        throw new InvalidOperationException($"Could not render report {reportIdentifier} with binding order {bindingOrder}.");
    }

    reportDictionary.GetOrAdd(bindingOrder, exportedImage);
}
var reportDictionary = new ConcurrentDictionary<int, string>();

List<Task> tasks = new List<Task>();
foreach (var report in request.Reports)
{
    tasks.Add(ProcessReport(report, reportDictionary));
}
await Task.WhenAll(tasks);
\$\endgroup\$
4
  • 1
    \$\begingroup\$ I think if you could extend your answer with some explanation why we should not mix these two tools that would give clarity for the OP. \$\endgroup\$ Commented Mar 18, 2021 at 10:05
  • 1
    \$\begingroup\$ @PeterCsala updated the answer. I think OP knows that, the concern in the question: it doesn't seem to work so this was essentially a workaround, i just showed a proper implementation. \$\endgroup\$
    – aepot
    Commented Mar 18, 2021 at 10:14
  • \$\begingroup\$ Thanks for the response. In my case, would you recommend using TPL or shall I only use async await pattern? \$\endgroup\$
    – Usman Khan
    Commented Mar 18, 2021 at 11:11
  • \$\begingroup\$ @UsmanKhan I guess you can do anything with async. Mixing tools makes the code less maintaineable. But find the difference between asynchronous and multithreading, totally different things. \$\endgroup\$
    – aepot
    Commented Mar 18, 2021 at 11:47

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.