Starting Tasks in Foreach Loop Uses Value of Last Item

Starting Tasks In foreach Loop Uses Value of Last Item

You're closing over the loop variable. Don't do that. Take a copy instead:

foreach (string path in paths)
{
string pathCopy = path;
var task = Task.Factory.StartNew(() =>
{
Boolean taskResult = ProcessPicture(pathCopy);
return taskResult;
});
// See note at end of post
task.ContinueWith(t => result &= t.Result);
tasks.Add(task);
}

Your current code is capturing path - not the value of it when you create the task, but the variable itself. That variable changes value each time you go through the loop - so it can easily change by the time your delegate is called.

By taking a copy of the variable, you're introducing a new variable each time you go through the loop - when you capture that variable, it won't be changed in the next iteration of the loop.

Eric Lippert has a pair of blog posts which go into this in a lot more detail: part 1; part 2.

Don't feel bad - this catches almost everyone out :(


Note about this line:

task.ContinueWith(t => result &= t.Result);

As pointed out in comments, this isn't thread-safe. Multiple threads could execute it at the same time, potentially stamping on each other's results. I haven't added locking or anything similar as it would distract from the main issue that the question is interested, namely variable capture. However, it's worth being aware of.

Identifying last loop when using for each

How about obtaining a reference to the last item first and then use it for comparison inside the foreach loop? I am not say that you should do this as I myself would use the index based loop as mentioned by KlauseMeier. And sorry I don't know Ruby so the following sample is in C#! Hope u dont mind :-)

string lastItem = list[list.Count - 1];
foreach (string item in list) {
if (item != lastItem)
Console.WriteLine("Looping: " + item);
else Console.Writeline("Lastone: " + item);
}

I revised the following code to compare by reference not value (can only use reference types not value types). the following code should support multiple objects containing same string (but not same string object) since MattChurcy's example did not specify that the strings must be distinct and I used LINQ Last method instead of calculating the index.

string lastItem = list.Last();
foreach (string item in list) {
if (!object.ReferenceEquals(item, lastItem))
Console.WriteLine("Looping: " + item);
else Console.WriteLine("Lastone: " + item);
}

Limitations of the above code. (1) It can only work for strings or reference types not value types. (2) Same object can only appear once in the list. You can have different objects containing the same content. Literal strings cannot be used repeatedly since C# does not create a unique object for strings that have the same content.

And i no stupid. I know an index based loop is the one to use. I already said so when i first posted the initial answer. I provided the best answer I can in the context of the question. I am too tired to keep explaining this so can you all just vote to delete my answer. I'll be so happy if this one goes away. thanks

Last iteration in foreach loop with Object Javascript

You could use index and array parameters to check if there is next element. You can also check if current index is equal length - 1 index == arr.length - 1

let objName = {

item1 : "someItem",

item2 : "someItem",

item3 : "someItem",

}

Object.keys(objName).forEach((item, index, arr) => {

console.log(item);

if(!arr[index + 1]) console.log('End')

});

Is reassigning a task value necessary when not using task.ContinueWith()

Ok, thanks to damien-the-unbeliever, for pointing me back to Eric Lippert's Blog: closing over the loop variable part two.

So the short answer is yes, if I am c# v 4.0 or earlier. It is unneeded in any case if at c# 5.0 or later.

Best way to wait for .forEach() to complete

If there is no asynchronous code inside the forEach, forEach is not asynchronous, for example in this code:

array.forEach(function(item){ 
//iterate on something
});
alert("Foreach DONE !");

you will see the alert after forEach finished.

Otherwise (You have something asynchronous inside), you can wrap the forEach loop in a Promise:

var bar = new Promise((resolve, reject) => {
foo.forEach((value, index, array) => {
console.log(value);
if (index === array.length -1) resolve();
});
});

bar.then(() => {
console.log('All done!');
});

Credit: @rolando-benjamin-vaz-ferreira

Find the last element of an array while using a foreach loop in PHP

It sounds like you want something like this:

$numItems = count($arr);
$i = 0;
foreach($arr as $key=>$value) {
if(++$i === $numItems) {
echo "last index!";
}
}

That being said, you don't -have- to iterate over an "array" using foreach in php.

Different Behaviour on parallel Task Execution with Removing Elements from a Concurrent Dictionary

for (int i = 0; i < 29; i++)
{
RemoveTakes.Add(Task.Run(() => RemoveUnit(management, units[i])));
}

doesn't do what you think it does.

You should change it to:

for (int i = 0; i < 29; i++)
{
var bob = i;
RemoveTakes.Add(Task.Run(() => RemoveUnit(management, units[bob])));
}

With your original code, by the time the Task ran the value of i had changed. So you need to store that in a separate variable (bob - scoped inside the loop) so that the first Task gets 0, the next gets 1 etc.

Your issue is basically the same as this one (although that uses a foreach the principle is the same).

Note that if you have Resharper installed it should have highlighted this issue for you.

How do you get the index of the current iteration of a foreach loop?

The foreach is for iterating over collections that implement IEnumerable. It does this by calling GetEnumerator on the collection, which will return an Enumerator.

This Enumerator has a method and a property:

  • MoveNext()
  • Current

Current returns the object that Enumerator is currently on, MoveNext updates Current to the next object.

The concept of an index is foreign to the concept of enumeration, and cannot be done.

Because of that, most collections are able to be traversed using an indexer and the for loop construct.

I greatly prefer using a for loop in this situation compared to tracking the index with a local variable.

Use a phing ForEach loop to execute tasks

At first I tried to use the loop variable as the target

<foreach list="${mylist}" param="item" target="${item"} />

but it doesn't seem to allow a variable as a target name

The target attribute of the foreach task is able to use variables as a value, but at this point param="item" is not yet available but in the target it is.

So I split it up into two tasks.

<foreach list="${parts}" param="dTarg" target="doIt" >
<task name="DoIt">
<phingcall target="build">
<property name="extension" value="${dTarg}" />
</phingcall>
</task>

Here you try to use a task task which is simply not a valid target.

What you want to do instead is to have targets to iterate over. The following example demonstrates the usage:

Input build.xml

<?xml version="1.0" encoding="utf-8" ?>

<project name="test" default="main">
<property name="mylist" value="A,B,C" />

<target name="main">
<foreach list="${mylist}" param="item" target="DoIt"/>
</target>

<target name="DoIt">
<echo>${item}</echo>
</target>

</project>

Output

test > main:

test > DoIt:

[echo] A

test > DoIt:

[echo] B

test > DoIt:

[echo] C

BUILD FINISHED

Complex Example (with property override and inheritAll)

<?xml version="1.0" encoding="utf-8" ?>

<project name="test" default="main">
<property name="mylist" value="A,B,C" />

<target name="main">
<foreach list="${mylist}" param="item" target="DoIt"/>
</target>

<target name="DoIt">
<phingcall target="${item}" inheritAll="true">
<property name="extension" override="true" value="${item}-ext" />
</phingcall>
</target>

<target name="A">
<echo>Inside target ${item} with ${extension} extension</echo>
</target>

<target name="B">
<echo>Inside target ${item} with ${extension} extension</echo>
</target>

<target name="C">
<echo>Inside target ${item} with ${extension} extension</echo>
</target>
</project>

Output

test > main:

test > DoIt:

test > A:

[echo] Inside target A with A-ext extension

test > DoIt:

test > B:

[echo] Inside target B with B-ext extension

test > DoIt:

test > C:

[echo] Inside target C with C-ext extension

BUILD FINISHED

Example execute as one task with changed values from the list

<?xml version="1.0" encoding="utf-8" ?>

<project name="test" default="main">
<property name="mylist" value="A,B,C" />

<target name="main">
<foreach list="${mylist}" param="item" target="DoIt"/>
</target>

<target name="DoIt">
<phingcall target="build">
<property name="extension" override="true" value="${item}-ext" />
</phingcall>
</target>

<target name="build">
<echo>Inside target build with ${extension}</echo>
</target>
</project>

Output

test > main:

test > DoIt:

test > build:

[echo] Inside target build with A-ext

test > DoIt:

test > build:

[echo] Inside target build with B-ext

test > DoIt:

test > build:

[echo] Inside target build with C-ext

BUILD FINISHED

Simplified and final build

<?xml version="1.0" encoding="utf-8" ?>

<project name="test" default="main">
<property name="mylist" value="A,B,C" />

<target name="main">
<foreach list="${mylist}" param="item" target="build">
<property name="extension" override="true" value="${item}-ext" />
</foreach>
</target>

<target name="build">
<echo>Inside target build with ${extension}</echo>
</target>
</project>

Output

test > main:

test > build:

[echo] Inside target build with A-ext

test > build:

[echo] Inside target build with B-ext

test > build:

[echo] Inside target build with C-ext

BUILD FINISHED

How to parallel process multiple async task foreach loop

Since you are not awaiting the call to GetIsConnected() in your AFTER example, the loop will continue and the value may or may not be checked against what it would be once the call to GetIsConnected() is finished, what I think you'd want to do is add another method with a return type of Task and let the method call GetIsConnected() and await it inside the method then call GetCarDetailsAsync() if necessary:

//changed to return type of Task, as async void should be avoided
//unless this is an event handler method
private async Task Refresh()
//private async void Refresh() //change to this if event handler
{
List<Task> listOfTasks = new List<Task>();
foreach (var item in Cars.ToList<Carnet>())
{
listOfTasks.Add(GetCarDetails(item));
}
await Task.WhenAll(listOfTasks).ConfigureAwait(false);
}

//method to await the call to GetIsConnect()
//and then call GetCarDetailsAsync() if necessary
private async Task GetCarDetails(Carnet c)
{
await c.GetIsConnected();
if (c.IsConnected)
{
await c.GetCarDetailsAsync();
}
}


Related Topics



Leave a reply



Submit