Установка анонимного метода обработчиком события в цикле foreach

#c_sharp #c_sharp_faq

Определяю класс-издатель события

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);
    }
}


, класс-подписчик

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


, тестирую

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 cars = new List();
    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();
}


Выдаёт

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


А хотелось бы

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


Почему так происходит? Как исправить?



Версия C# 3.0 .Net 3.5
    

Ответы

Ответ 1

Это - особенность старой версии языка. Переменная цикла захватывается по ссылке,
а не по значению - и потому к моменту возникновения события указывает на последнего
водителя.

Решений тут три:


Если возможно, перейдите на современную версию языка. Это проще, чем вы думаете:
Visual Studio Community Edition бесплатна для любого личного использования, обучения
или работы над открытыми проектами.
Скопируйте значение переменной цикла в локальную переменную:

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

Смените тип коллекции drivers с массива на список (List) - тогда вы получите
метод ForEach, принимающий делегат:

drivers.ForEach(driver => {
  //...
});


Ответ 2

Небольшое уточнение/дополнение к правильному ответу @Pavel Mayorov.

В C# до 5-ой версии цикл 

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


раскрывался примерно в такую конструкцию:

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


Поэтому все замыкания видели одну и ту же переменную driver, которая менялась с каждой
итерацией цикла. И значит, при приходе события Started у этой переменной было уже «финальное»
значение.

Начиная с версии 5.0, цикла стал раскрываться в другую конструкцию:

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


И значит, каждое замыкание теперь видит свою переменную driver, только для этой итерации.
Таким образом, в C# 5+ ваш код будет вести себя ожидаемым образом.



Дополнительное чтение по теме: Closing over the loop variable considered harmful.


Ответ 3

Ваш delegate захватывает переменную driver (это именно closure), а когда машины стартуют,
то в ней находится последний в списке водитель, он же Вася. Как уже посоветовали, попробуйте
"внутри форича скопипастить водителя в переменную" (лучше прямо driver.Name) и посмотрите
на результат. Правильнее всего, конечно, в Car иметь ссылку на Driver