Setting an anonymous method as an event handler in a foreach loop

Question

I define event class publisher

class Car { public string Name { get; set; } public Car(string name) { Name = name; } public event EventHandler Started; public void Start() { if (Started != null) Started(this, EventArgs.Empty); } }

subscriber class

 class Driver { public string Name { get; set; } public Driver(string name) { Name = name; } }

I'm testing

 static void Main(string[] args) { var fomenko = new Driver("Фоменко"); var shumaher = new Driver("Шумахер"); var vasya = new Driver("Вася"); Driver[] drivers = new Driver[] { fomenko, shumaher, vasya }; List<Car> cars = new List<Car>(); foreach (var driver in drivers) { var car = new Car ( driver == fomenko ? "Маруся" : driver == shumaher ? "Ф1" : "Запорожец" ); car.Started += delegate(object o, EventArgs ea) { Console.WriteLine("Стартовала машина {0} с пилотом {1}", car.Name, driver.Name); }; cars.Add(car); } foreach (var car in cars) { car.Start(); } Console.ReadKey(); }

Gives out

 Стартовала машина Маруся с пилотом Вася Стартовала машина Ф1 с пилотом Вася Стартовала машина Запорожец с пилотом Вася

I would like to

 Стартовала машина Маруся с пилотом Фоменко Стартовала машина Ф1 с пилотом Шумахер Стартовала машина Запорожец с пилотом Вася

Why it happens? How to fix?

Version C # 3.0 .Net 3.5

The logic becomes much simpler and there will be less problems.
There is something about closures, but I don’t understand them =)
Possible duplicate question: Why does a delegate from a separate thread add incorrect data to a UI thread?
Somewhere there was a more canonical question, but I could not find it.
@Monk, I'll ever learn how to make a good example of the problem)) this time it didn't work out.
It is necessary to solve the problem of subscription to events in a loop, using variables visible in the loop block.
@andreycha, the problem of the link is probably similar, and maybe even one in its essence.
But because of multithreading, it is very difficult to understand the solution in my case.

Pavel Mayorov Pavel Mayorov 48.9k five 50 110 · Accepted Answer · 2016-10-28T05:41:04

This is a feature of the old version of the language. The cycle variable is captured by reference, and not by value - and therefore by the time the event occurs indicates the last driver.

There are three solutions here:

If possible, upgrade to a modern version of the language. It's easier than you think: Visual Studio Community Edition is free for any personal use, training, or work on open source projects.

Copy the value of the loop variable to a local variable:

 foreach (var driver in ...) { var driver2 = driver; //... }

Change the type of the drivers collection from an array to a list ( List<Driver> ) - then you will get the ForEach method, the receiving delegate:
```
 drivers.ForEach(driver => { //... }); 
```

It may make sense to add text about what has changed in the fresh versions of the language.
It seems to me, it is obvious to us with you, but not to beginners.
The loop variable is captured by reference, it is just a new variable every time.
You can check it by capturing it in two lambdas and changing the value in one of the lambdas.

Answer 2 · 2016-10-28T10:24:41

A small clarification / addition to the correct answer @Pavel Mayorov.

In C # up to version 5 cycle

 foreach (var driver in drivers) { // тело цикла }

revealed something like this:

 using (var en = drivers.GetEnumerator()) { Driver driver; // вне цикла while (en.MoveNext()) { driver = en.Current; // тело цикла } }

Therefore, all closures saw the same driver variable, which changed with each iteration of the loop. And that means that when the Started event Started , this variable already had a “final” value.

Starting from version 5.0, the cycle began to unfold into another structure:

 using (var en = drivers.GetEnumerator()) { while (en.MoveNext()) { Driver driver = en.Current; // внутри цикла // тело цикла } }

And that means that each closure now sees its driver variable, only for this iteration. Thus, in C # 5+, your code will behave as expected.

Additional reading on the topic: Closing over loop is considered harmful .

Alexander Alexeev Alexander Alexeev 88 four · Answer 3 · 2016-10-27T18:56:06

Your delegate captures the driver variable (this is precisely closure), and when the cars start, then the last driver in the list is in it, which is Vasya. As already advised, try to "copy the driver into a variable inside forich" (better driver.Name is better) and look at the result. The most correct, of course, in Car is to have a link to the Driver

It is not entirely clear how the term " copy-paste " is applied to the reference object.

Setting an anonymous method as an event handler in a foreach loop

3 answers 3

More articles: