views:

373

answers:

4

I am dynamically adding a bunch of controls to a form. Each control calls the same method, and in that method I need to know the array index of the the control that performed the action.

CheckBox[] myCB = new CheckBox[100];
int i;
for (i = 0; i < 100; i++)
{
    myCB[i] = new CheckBox();
    myCB[i].Text = "Clicky!";
    myCB[i].Click += new System.EventHandler(dynamicbutton_Click);
    tableLayoutPanel1.Controls.Add(myCB[i]);
}

private void dynamicbutton_Click(Object sender, System.EventArgs e)
{
    label1.Text = sender.???array index property???.ToString();
}

So if I click myCB[42] label1 will read "42" Of course, if there is an easier way to handle dynamic controls I'd appreciate pointers.

+3  A: 

Control's should have a Tag property. Maybe you can attach the index to the Tag. You will incur boxing though...

BFree
If he's explicitly looking for the index rather than an arbitrary ID/indicator, then this probably isn't the best way.
Noldorin
Well he did explicitly state: "Of course, if there's an easier way to handle dynamic controls I'd appreciate pointers...." Finding the index each time is horribly inefficient IMO.
Jon Skeet
Given that the code is being executed on a button click, and that there are only 100 to check through, efficiency is hardly going to be a big issue!
Noldorin
But yeah, this method does have its advantages, of course.
Noldorin
In particular, it associates the information with the object itself, which I view as a cleaner solution.
Jon Skeet
+5  A: 
private void dynamicbutton_Click(Object sender, System.EventArgs e)
{
    label1.Text = Array.IndexOf(myCB, (CheckBox)sender).ToString();
}
SLaks
This is the simplest answer, although not efficient for large number of checkboxes. For better performance it's probably better to use the Tag property of the checkbox.
rein
This is the correct answer, but I think that Jon and Rein are right - tags are a better general solution for this problem than finding the index.
Adam Davis
+2  A: 

One obvious solution would be to set the tag:

CheckBox[] myCB = new CheckBox[100];
for (int i = 0; i < myCB.Length; i++)
{
    myCB[i] = new CheckBox();
    myCB[i].Text = "Clicky!";
    myCB[i].Click += new System.EventHandler(dynamicbutton_Click);
    myCB[i].Tag = i;
    tableLayoutPanel1.Controls.Add(myCB[i]);
}

Then:

private void dynamicbutton_Click(Object sender, System.EventArgs e)
{
    Control control = (Control) sender;
    label1.Text = sender.Tag.ToString();
}

Another alternative is to capture the information in the event handler, most simply using a lambda expression or anonymous method:

CheckBox[] myCB = new CheckBox[100];
for (int i = 0; i < myCB.Length; i++)
{
    int index = i; // This is very important, as otherwise i will
                  // be captured for all of them
    myCB[i] = new CheckBox();
    myCB[i].Text = "Clicky!";
    myCB[i].Click += (s, e) => label1.Text = index.ToString();
    tableLayoutPanel1.Controls.Add(myCB[i]);
}

or for more complicated behaviour:

CheckBox[] myCB = new CheckBox[100];
for (int i = 0; i < myCB.Length; i++)
{
    int index= i; // This is very important, as otherwise i will
                  // be captured for all of them
    myCB[i] = new CheckBox();
    myCB[i].Text = "Clicky!";
    myCB[i].Click += (s, e) => DoSomethingComplicated(index, s, e);
    tableLayoutPanel1.Controls.Add(myCB[i]);
}

(where you declare DoSomethingComplicated appropriately).

Jon Skeet
Why do you need to declare a new local variable, aren't you just assigning a value which is changing each time? I mean, index.ToString() returns a new string instance, doesn't it?
Vinko Vrsalovic
Without the new local variable, the "i" variable is captured: not the value, but the variable itself. That means when you clicked on the label, it would set the value to 100 every time. Try it! This is a well-known issue with captured variables and loops.
Jon Skeet
Will the issue be solved in 4.0? And what's its cause?
Vinko Vrsalovic
This is a feature, not a bug.See http://en.wikipedia.org/wiki/Closure_(computer_science)
SLaks
I'd say it's a feature in the "for" loop, but a flaw in language design for the "foreach" loop: it certainly *looks* like a new variable is being created in each declaration of the loop with foreach. (See http://stackoverflow.com/questions/969729 for more discussion on this.)
Jon Skeet
@SLaks, it has nothing to do directly with closures, but to whether a loop creates a new local variable on each iteration or it reuses a (n implicit) variable declared just outside the loop. Closures will manifest the same 'problem' if you use a while and a variable declared outside.
Vinko Vrsalovic
@Vinko: It absolutely has to do with closures. It's only because it's a closure that the *variable* is captured rather than just its value at the point of the delegate creation.
Jon Skeet
+3  A: 
int j = i;
myCB[i].Click += delegate(object sender, EventArgs e) {
 // here you can use "j"

};
John Saunders
Ooh, I like the use of closures to do this - that's pretty novel.
Noldorin
you could use i, but i would be 100. You would want to declare a new variable "int j = i; then use j within the delegate.
Timothy Carter
To do this, you MUST declare a separate variable inside the loop and assign it to i.`i` itself will be equal to 100 in all of the delegates, which is not what you want.
SLaks
Downvoted until the use of "i" is changed to reflect SLaks comment. See my answer for an example of how to do this safely.
Jon Skeet
@Vinko: thanks for the edit.
John Saunders
@Jon: do I get my 2 points back even though someone else did the edit?
John Saunders