Why does my delegate only use the last item from my foreach loop?

Question

Scenario: I am building a scheduling system and each timer event I wanted to run a custom method instead of the usual `Timer.Elapsed` event. So I wrote something like this. ``` foreach (ScheduleElement schedule in schedules) { TimeSpan timeToRun = CalculateTime(schedule); schedule.Timer = new Timer(timeToRun.TotalMilliseconds); schedule.Timer.Elapsed += delegate { Refresh_Timer(schedule); }; schedule.Timer.AutoReset = true; schedule.Timer.Enabled = true; } ``` Ok so simple enough that actually did create my timers. However, I wanted each elapsed event to run using the schedule element that it passed in. My question is, why does the Elapsed event only pass in the last ScheduleElement in the for loop for every single Timer.Elapsed event. Now I know what fixes it, I am just not sure why. If I roll back to the original Timer.Elapsed event and extend the Timer class with my own class I can work around it. Like so. The Fix: ``` foreach (ScheduleElement schedule in schedules) { TimeSpan time…

Accepted Answer

This is because you're using closure in your delegate, and it closes over the same variable, which is shared for each iteration of the foreach loop.

For details, see Eric Lippert's article Closing over the loop variable considered harmful.

In this case, you can easily fix it with a temporary:

```
foreach (ScheduleElement schedule in schedules) {
    TimeSpan timeToRun = CalculateTime(schedule);
    schedule.Timer = new Timer(timeToRun.TotalMilliseconds);

// Make a temporary variable in the proper scope, and close over it instead
    var temp = schedule;
    schedule.Timer.Elapsed += delegate { Refresh_Timer(temp); };
```

Note that C# 5 changes this behavior for `foreach` loops. If you compile this with the latest compilers, the issue no longer exists.

Why does my delegate only use the last item from my foreach loop?

Solution

Problem

Related problems