Cannot convert from 'System.Threading.Tasks.Task' to 'System.Collections.Generic.Dictionary'

Question

I believe I might just have the syntax wrong but what I'm trying to do is create a task that runs after another task is finished.

I have a task for each array of 100 in a list. It starts a new thread passing that array into a method. The method returns a dictionary when it finishes. I'm trying to create a task to run after the method is finishes where it passes the returned dictionary to a separate method that does some more work.

static void Main(string[] args)
    {
        try
        {
            stopwatch = new Stopwatch();
            stopwatch.Start();
            while (true)
            {
                startDownload();
            }
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }
    }

public static async void startDownload()
    {
        try
        {

            DateTime currentDay = DateTime.Now;

            if (Helper.holidays.Contains(currentDay) == false)
            {
                List<string> markets = new List<string>() { "amex", "global", "nasdaq", "nyse" };

                Parallel.ForEach(markets, async market =>
                {
                    try
                    {

                        IEnumerable<string> symbolList = Helper.getStockSymbols(market);
                        var historicalGroups = symbolList.Select((x, i) => new { x, i })
                          .GroupBy(x => x.i / 100)
                          .Select(g => g.Select(x => x.x).ToArray());

                        Task<Dictionary<string, string>>[] historicalTasks =
                                                           historicalGroups.Select(x => Task.Run(() =>
                                                           Downloads.getHistoricalStockData(x, market)))
                                                                    .ToArray();

                        Dictionary<string, string>[] historcalStockResults = await
                                                                             Task.WhenAll(historicalTasks);

                        foreach (var dictionary in historcalStockResults)
                        {
                            Downloads.updateSymbolsInDB(dictionary);
                        }
                    }
                    catch (Exception ex)
                    {
                        Console.WriteLine(ex.Message);
                    }
                });

                await Task.Delay(TimeSpan.FromHours(24));
            }
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }
    }

Are you after an array of `Task>` or an array of `Dictionary`? — Mike Guthrie, Dec 25 '14 at 06:13
public static async Task> getHistoricalStockData(string[] symbols, string market) I need to pass in the Task> — DarthVegan, Dec 25 '14 at 06:16
@MikeGuthrie The updateSymbols method looks like this and returns void public static void updateSymbolsInDB(Dictionary errors) — DarthVegan, Dec 25 '14 at 06:17
Will `updateSymbolsInDB` run for each result of `getHistoricalStockData`, or is it supposed to run only once on the aggregate of all `getHistoricalStockData` results? — Mike Guthrie, Dec 25 '14 at 06:22
@MikeGuthrie I was trying to have it run only once because I thought that would be more efficient — DarthVegan, Dec 25 '14 at 06:23

score 5 · Accepted Answer · edited May 23 '17 at 12:16

5

I would advise not to use ContinueWith at all if you're already using await. The reason is the verbosity you end up with in your code.

Instead, use await where possible. The code ends up like this:

var historicalGroups = symbolList
                       .Select((x, i) => new { x, i })
                       .GroupBy(x => x.i / 100)
                       .Select(g => g.Select(x => x.x).ToArray());

var historicalTasks = historicalGroups.Select(x => Task.Run(() => 
                                       Downloads.getHistoricalStockData(x, market)))
                                      .ToArray();

var historcalStockResults = await Task.WhenAll(historicalTasks);

foreach (var dictionary in historcalStockResults)
{
    Downloads.updateSymbolsInDB(dictionary);
}

Note the use of Task.Run instead of Task.Factory.StartNew. You should use that instead. More on that here

Edit:

If you need to execute this code once every 24 hours, add a Task.Delay and await on it :

await Task.Delay(TimeSpan.FromHours(24));

Edit 2:

The reason your code isn't working is because startDownload is async void, and you're not awaiting on it. Thus your while loop keeps iterating regardless of your Task.Delay.

Because you're inside a Console Application, you can't await because Main methods cant be async. So to work around that, change startDownload to be async Task instead of async void, and Wait on that returned Task. Do note that using Wait should almost never be used, expect for special scenarios (such as the one while running inside a console app):

public async Task StartDownload()

and then

while (true)
{
    StartDownload().Wait();
}

Also note that mixing Parallel.Foreach and async-await isn't always the best idea. You can read more on that in Nesting await in Parallel.ForEach

edited May 23 '17 at 12:16

Community

1
1

answered Dec 25 '14 at 06:52

Yuval Itzchakov

146,575
32
257
321

@user3610374 I had answered before I fully understood what you were after, thus how I answered. *This* however is the answer I'd recommend, as it ends up with much more readable code. If you still only wanted the single `Downloads.updateSymbolsInDB` call, you could replace the `foreach` block with the `Aggregate` logic as I had included in my answer. But again, profile first to see if that is even beneficial. – Mike Guthrie Dec 25 '14 at 07:01
I forgot to mention/ask one important thing. I have this code running in a loop and it is supposed to run again after 24 hours but I'm using Task.Delay(TimeSpan.FromHours(24)).Wait(); and this doesn't actually wait 24 hours. It keeps running the code over and over again. Is there a better way to run this with the one line method you mentioned? – DarthVegan Dec 25 '14 at 07:03
Why are you using `Wait`? If you have this code running in some `while` loop, simply use `await Task.Delay(TimeSpan.FromHours(24));` – Yuval Itzchakov Dec 25 '14 at 07:05
I tried that also but neither one is working. I inserted a breakline inside the while loop, and it keeps calling the method and running the code which isn't what I want. I believe await is a non blocking call so it just continues right? – DarthVegan Dec 25 '14 at 07:11
Since all I'm doing is downloading data and saving it to a database as quickly as possible, should I just stick with TPL or use TPL Dataflow or just stick with async and await with the tasks? – DarthVegan Dec 25 '14 at 07:29
It really depends on the job. If you're doing async IO, such as database calls, you can simply use `async-await`, no need for extra threading. If you're mixing async IO bound work with CPU bound work, i'd recommend using TPL dataflow. – Yuval Itzchakov Dec 25 '14 at 07:31

Mike Guthrie · Answer 2 · 2014-12-25T07:05:56.137

You'll see that ContinueWith takes as Task as argument.

Also, from working through your logic in our comments, it looks like you'll need to update your code. You are trying to run the Downloads.updateSymbolsInDB once on the result of all WhenAll tasks are completed. In your case it looks like you'll need

await Task<Dictionary<string, string>>
    .WhenAll(historicalGroups.Select(g => Task.Factory.StartNew(() => Downloads.getHistoricalStockData(g, market))))
    .ContinueWith((i) => Downloads.updateSymbolsInDB(i.Result.Aggregate((agg, next) =>
            {
                foreach (var p in next)
                {
                    if (!agg.ContainsKey(p.Key)) { agg.Add(p.Key, p.Value); }
                }
                return agg;
            })));

Note that I also used the Task<TResult> to initiate, so that the ContinueWith is strongly typed to provide the Dictionary<string, string> i.Result.

Also note that you'll want to revisit what I'm doing in the i.Result.Aggregate logic. You'll need to update that to be correct to your case. Additionally, it may be worth revisiting to see if it is more efficient to just call Downloads.updateSymbolsInDB multiple times, or if the Aggregate call is the better choice.

(Last note: the end result is a void, so the await isn't assigned.)

In review, there were a number of problems with your code (meant as a statement, not an accusation - no offense intended). Your tasks created via Task.Factory.StartNew for your WhenAll method had no return value, thus were just basic Task objects, and were in fact throwing away the results of the Downloads.getHistoricalStockData work. Then, your ContinueWith needed to work with the Task<TResult> of the WhenAll, where TResult will be an array of the return value of each task within WhenAll.

EDIT: As was asked how the code could be updated if Downloads.updateSymbolsInDB were to be called multiple times, it could simply be done as:

IEnumerable<string> symbolList = Helper.getStockSymbols(market);
var historicalGroups = symbolList.Select((x, i) => new { x, i })
    .GroupBy(x => x.i / 100)
    .Select(g => g.Select(x => x.x).ToArray());
Parallel.ForEach(historicalGroups, g => Downloads.updateSymbolsInDB(Downloads.getHistoricalStockData(g, market)));

Yeah I thought so as well but it gives me the error that Task doesn't contain a definition for Result — DarthVegan, Dec 25 '14 at 06:00
I updated my code and I'm still getting the error that Task doesn't contain a definition for Result — DarthVegan, Dec 25 '14 at 06:03
What would the correct code look like if I wanted to call the method multiple times? — DarthVegan, Dec 25 '14 at 06:40

Cannot convert from 'System.Threading.Tasks.Task' to 'System.Collections.Generic.Dictionary'

2 Answers2