c# await in for loop

Question

I have a list of items that need to be updated if information is missing. However, I'm making a call to Google Location services to do that. I'm wondering how I can append the necessary Lat & Long information asynchronously if possible

my code

public static void PullInfo() { foreach (var item in SAPItems) { if(item.MDM_Latitude == null || item.MDM_Longitude == null) { var point = GetMapPoint(item.AddressLine1 + " " + item.FiveDigitZip); item.MDM_Latitude = point.Result.Latitude.ToString(); item.MDM_Longitude = point.Result.Longitude.ToString(); } } foreach(var item in SAPItems) Console.WriteLine(item.MDM_Latitude + " " + item.MDM_Longitude); } private static async Task<MapPoint> GetMapPoint(string add) { var task = Task.Run(() => LocationService.GetLatLongFromAddress(add)); return await task; }

@Rup Just set the numbers in that specific item object. I updated the question. — BeLEEver
– BeLEEver, Commented Aug 29, 2018 at 17:01
Why are you using .Result when you await your GetMapPoint function? After the await, you already have the data, just await there, not inside GetMapPoint. — xxbbcc
– xxbbcc, Commented Aug 29, 2018 at 17:02
Try changing the List to a ConcurrentBag and the foreach to a Parallel.ForEach — ste-fu
– ste-fu, Commented Aug 29, 2018 at 17:03
why can't you just say var point = await GetMapPoint(item.AddressLine1 + " " + item.FiveDigitZip); — Rahul
– Rahul, Commented Aug 29, 2018 at 17:08

Guillermo Gutiérrez · Accepted Answer · 2018-08-29 17:49:05Z

You can get multiple map points asynchronously with multiple tasks (notice that it requires to convert PullInfo() to async-await):

public static async Task PullInfo() { // Create tasks to update items with latitude and longitude var tasks = SAPItems.Where(item => item.Latitude == null || item.Longitude == null) .Select(item => GetMapPoint(item.AddressLine1 + " " + item.FiveDigitZip) .ContinueWith(pointTask => { item.MDM_Latitude = pointTask.Result.Latitude.ToString(); item.MDM_Longitude = pointTask.Result.Longitude.ToString(); })); // Non-blocking await for tasks completion await Task.WhenAll(tasks); // Iterate to append Lat and Long foreach(var item in SAPItems) Console.WriteLine(item.MDM_Latitude + " " + item.MDM_Longitude); } private static Task<MapPoint> GetMapPoint(string add) { return Task.Run(() => LocationService.GetLatLongFromAddress(add)); }

If PullInfo() cannot be converted to async-await, you can force the thread to wait for the results, but it will block the current thread:

public static void PullInfo() { // Create tasks to update items with latitude and longitude var tasks = SAPItems.Where(item => item.Latitude == null || item.Longitude == null) .Select(item => GetMapPoint(item.AddressLine1 + " " + item.FiveDigitZip) .ContinueWith(pointTask => { item.MDM_Latitude = pointTask.Result.Latitude.ToString(); item.MDM_Longitude = pointTask.Result.Longitude.ToString(); })); // Wait for tasks completion (it will block the current thread) Task.WaitAll(tasks.ToArray()); // Iterate to append Lat and Long foreach(var item in SAPItems) Console.WriteLine(item.MDM_Latitude + " " + item.MDM_Longitude); } private static Task<MapPoint> GetMapPoint(string add) { return Task.Run(() => LocationService.GetLatLongFromAddress(add)); }

Running example of this last code sample: https://ideone.com/0uXGlG

I've run this method, but my missing latitudes and longitudes are still not being assigned. Any thoughts?
@user3434042 The first or the second one? Do you want to update Latitude/Longitude or MDM_Latitude/MDM_Longitude?
tasks.ToArray() will cause the enumerable to be iterated. No need for the last foreach loop.

xxbbcc · Accepted Answer · 2018-08-29 17:07:37Z

You just need to await the call to getting the data (notice how the await got moved from GetMapPoint):

public static async Task PullInfo() { foreach (var item in SAPItems) { if(item.Latitude == null || item.Longitude == null) { var point = await GetMapPoint(item.AddressLine1 + " " + item.FiveDigitZip); item.MDM_Latitude = point.Latitude.ToString(); item.MDM_Longitude = point.Longitude.ToString(); } } foreach(var item in SAPItems) Console.WriteLine(item.MDM_Latitude + " " + item.MDM_Longitude); } private static Task<MapPoint> GetMapPoint(string add) { var task = Task.Run(() => LocationService.GetLatLongFromAddress(add)); return task; }

You're not modifying your SAPItems collection, just each individual item. Once you have the response, you can just update the then-current item in the loop.

Putting await in the middle of the loop will compile to a really messed up state machine, and will break the intended logic. The answer Guillermo gave is the way to do it.
My apologies, thinking about this further (but I haven't tested), it shouldn't break the logic, but the state machine does compile to something way more complicated than it needs to be. Use ILSpy (or whatever you use) to see how your proposed method compiles compared to the accepted answer. In particular, look at the compiler generated MoveNext() method. If one was to implement this before C#5, I doubt someone would be tempted to create such logic. To me, you should nor write code that compiles to something you would never code without the syntactic sugar...but that's just me :-)
@Nik meh, I don't much care about the compiler's problems :) I agree that Guillermo's answer is more efficient because it has more parallelism - I just fixed the original code as is - he reworked it to a greater degree. I'm sitting in conference calls all day today :D

Collectives™ on Stack Overflow

c# await in for loop

2 Answers 2

3 Comments

4 Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

4 Comments

Linked

Related