Why is my delegate using only the last item from my foreach loop?

Question

Scenario: I create a scheduling system and every timer event. I wanted to run my own 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; } 

Well so simple that actually created my timers. However, I wanted each past event to be triggered using the schedule item in which it took place. My question is why the Elapsed event only passes the last ScheduleElement in the for loop for each individual Timer.Elapsed event.

Now I know this fixes, I just don't know why. If I go back to the original Timer.Elapsed event and extend the Timer class with my own class, I can get around it. Thus.

Correction:

 foreach (ScheduleElement schedule in schedules) { TimeSpan timeToRun = CalculateTime(schedule); schedule.Timer = new TimerEx(timeToRun.TotalMilliseconds); schedule.Timer.Elapsed +=new System.Timers.ElapsedEventHandler(Refresh_Timer); schedule.Timer.Tag = schedule; schedule.Timer.AutoReset = true; schedule.Timer.Enabled = true; } 

Then I drop the object sender back to my original object and remove the Tag property, which gives me my correct schedule for each unique timer.

So, why use delegate { } only ScheduleElement in the last ScheduleElement in the foreach loop for all timers?

EDIT 1

Class timer

 public TimerEx : Timer { public TimerEx(double interval) : base(interval) { } private Object _Tag; public Object Tag { get { return _Tag; } set { _Tag = value; } } } 

Answer 1

This is because you use closure in your deletion and close the same variable that is used for each iteration of the foreach loop.

For more information see Eric Lippert's article Closing a loop variable that is 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 modifies this behavior for foreach loops. If you compile this with the latest compilers, the problem no longer exists.