7

Can this be done in a for loop?

 TickEventArgs targs1 = new TickEventArgs(lbl1_up_time, _elapsedTime_up1); timer_up1.Tick += (sender, e) => Tick(targs1); TickEventArgs targs2 = new TickEventArgs(lbl2_up_time, _elapsedTime_up2); timer_up2.Tick += (sender, e) => Tick(targs2); TickEventArgs targs3 = new TickEventArgs(lbl3_up_time, _elapsedTime_up3); timer_up3.Tick += (sender, e) => Tick(targs3); TickEventArgs targs4 = new TickEventArgs(lbl4_up_time, _elapsedTime_up4); timer_up4.Tick += (sender, e) => Tick(targs4); TickEventArgs targs5 = new TickEventArgs(lbl5_up_time, _elapsedTime_up5); timer_up5.Tick += (sender, e) => Tick(targs5);

This doesnt work because i is out of bounds (5)

 targs[0] = new TickEventArgs(lbl1_up_time, _elapsedTime_up1); targs[1] = new TickEventArgs(lbl2_up_time, _elapsedTime_up2); targs[2] = new TickEventArgs(lbl3_up_time, _elapsedTime_up3); targs[3] = new TickEventArgs(lbl4_up_time, _elapsedTime_up4); targs[4] = new TickEventArgs(lbl5_up_time, _elapsedTime_up5); timers[0] = timer_up1; timers[1] = timer_up2; timers[2] = timer_up3; timers[3] = timer_up4; timers[4] = timer_up5; int i = 0; for (i = 0; i <= 4; i++) { timers[i].Tick += (sender, e) => Tick(targs[i]); }
Improve this question
3
  • This is coming from the lambda expression; i is shared between all of them. By the time the function is executed they're essentially being called like timers[i].Tick += (sender, e) => Tick(targs[5]). Declare a local int locali = i and use that in your line instead. Commented Aug 24, 2013 at 23:19
  • @ChrisSinclair post it as an answer. Commented Aug 24, 2013 at 23:20
  • possible duplicate of C# Captured Variable In Loop Commented Nov 2, 2013 at 6:25

2 Answers 2

Reset to default
10

This is coming from the lambda expression; i is shared between all of them. By the time the function is executed they're essentially being called like timers[i].Tick += (sender, e) => Tick(targs[5]).

To avoid this, create a locally scoped variable (int locali = i) and use that in your line instead. This will make sure that each lambda expression actually gets the value you expect.

for (i = 0; i <= 4; i++) { int locali = i; timers[locali].Tick += (sender, e) => Tick(targs[locali]); }

i becomes 5 from the last iteration of your loop before exiting. Naturally, you don't have a targs[5] element, so it throws an IndexOutOfRangeException.

Technically, you don't need to use locali for the timers[i].Tick part since it's evaluated immediately, but I personally find it confusing to mix the two.

Some additional reading on the concepet:

The foreach identifier and closures

Closing over the loop variable considered harmful

Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

Thanks for the quick answer. Works as it should now. Btw, do you know if there is a way to simplify the assignment of arrays, i.e all targs would be done in one statement?
@dirtyw0lf: But you have them all assigned to different timers. Did you really want just a single timer to clear the entire targs array?
No, just wondering if I could do it in one pass for all timers for array assignment
@dirtyw0lf: You mean skip the for loop and have a single line like this: timers[magic].Tick += (sender, e) => Tick(targs[magic])? I don't think so. You could create a separate object to handle all the Timer/TickEventArgs instances for you so you have a clean API that you then just have a single event that you can assign an Action<TimerEventArgs> delegate to.
@dirtyw0lf: Ahh. Possibly with reflection, but it'd be messy. You could also store a collection of the controls so you can just look them up like up_time_labels[X] and elapsedTimeUps[X], but you'd still have to populate those collections in a similar manner as you're doing now. What you got isn't too bad anyway; hard to really suggest more improvements without a better understanding of the full scope and usage of your code. Maybe you can post something in codereview.stackexchange.com
|
8

There is only one i in this case and all of the lambdas are capturing the same value. Use a local that is scoped to the loop so that each lambda has a different copy 

for (i = 0; i <= 4; i++) { int j = i; timers[j].Tick += (sender, e) => Tick(targs[j]); }
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.