Problem passing a Valuetype to a Delegate?

[james]

Hi guys,

I create a delegate and pass in a local variable. When the variable
is a reference type everything works fine, but when it is a valuetype
the delegate uses the value of the last instance that was created
(sorry that was a mouthful). Heres an example of the problem:

private void dlgFavorites_Load(object sender, EventArgs e)
{
Hashtable bookmarks =
ConfigurationSettings.GetConfig("Bookmarks") as Hashtable;

if(bookmarks == null)
return;

int i = 0;
foreach (DictionaryEntry pair in bookmarks)
{
Button btn = new Button();
btn.DialogResult = DialogResult.OK;
btn.Text = (string)pair.Key;

btn.Click += new EventHandler(delegate
{
m_selectedUrl = (string)pair.Value;
});
btn.Dock = DockStyle.Fill;

tableLayoutPanel1.Controls.Add(btn, (int)(i/2), i%2);

++i;
}
}

When I ran that, no matter which button was pressed, m_selectedUrl
equaled the value of the last DictionaryEntry in bookmarks. Any
ideas?

Thanks,
James

 

[Jon Skeet [C# MVP]]

I create a delegate and pass in a local variable. When the variable
is a reference type everything works fine, but when it is a valuetype
the delegate uses the value of the last instance that was created
(sorry that was a mouthful). Heres an example of the problem:

<snip>

I believe the problem is that "pair" becomes a captured variable, and
its scope is the whole "foreach" loop. If you try changing the delegate
part to:

string value = (string)pair.Value;
btn.Click += new EventHandler(delegate
{
m_selectedUrl = value;
});

I think you'll find it's okay.

Here's a short but complete program showing the same kind of thing in
effect with a for loop:

using System;

class Test
{
delegate void SimpleDelegate();

static void Main()
{
SimpleDelegate[] delegates = new SimpleDelegate[10];

for (int i=0; i < delegates.Length; i++)
{
delegates = delegate { Console.WriteLine(i); };
}

foreach (SimpleDelegate dg in delegates)
{
dg();
}
}
}

That prints "10" each time, because the "i" variable is effectively
shared between all the delegate instances. Change it so that the scope
of the variable used is *within* the loop, and it's fine:

using System;

class Test
{
delegate void SimpleDelegate();

static void Main()
{
SimpleDelegate[] delegates = new SimpleDelegate[10];

for (int i=0; i < delegates.Length; i++)
{
int j = i;
delegates = delegate { Console.WriteLine(j); };
}

foreach (SimpleDelegate dg in delegates)
{
dg();
}
}
}

That prints 0 to 9.

 

[james]

Thanks Jon that clears things up. I'm still trying to wrap my brain
around what the runtime is doing, but it makes sense nonetheless.

-James

 

[Jon Skeet [C# MVP]]

Thanks Jon that clears things up. I'm still trying to wrap my brain
around what the runtime is doing, but it makes sense nonetheless.

I think the word "scope" was probably not the most helpful one to use -
think about it in terms of the *lifetime* of the variable. The loop
variable is "alive" for the whole loop, and so it's shared between all
the delegates. Effectively, it's only declared once. The variables
within the loop block, however, are effectively "redeclared" each time
you go through the loop - so the variable on the first iteration of the
loop is a different variable to the one in the second iteration of the
loop.