views:

178

answers:

1

I am baffled by this simple task i do over and over again.

I have an array of child forms. The array is initiated in another form's constructor:

frmChildren = new ChildGUI[20];

When the user requests to see a child form, i do this:

if (frmChildren[nb] == null)
{
    frmChildren[nb] = new ChildGUI();
    frmChildren[nb].MdiParent = this.MdiParent;
}
frmChildren[nb].Show();

So far this works. In the background i can download new content for these forms. When a download is finished i fire a ChildChange event. Here is where it stops working. I simply want to close/hide any forms open then regenerate a new set of -frmChildren = new ChildGUI[20];- here is one of many trials:

        for (int i = 0; i < frmChildren.Length;i++ )
        {
            if (frmChildren[i] != null)
            {
                //frmChildren[i].BeginInvoke(new EventHandler(delegate
                //{
                    frmChildren[i].Close();
                //}));
            }
        }             
        frmChildren= new ChildGUI[20];

I get a Cross Thread exception on the .Close(). Notice i've already tried doing an invoke, but doing so bypasses the !=null for some reason. I think it may have something to do with the garbage collector. Anybody have an input?

+6  A: 

The problem is that your anonymous method is capturing i - so by the time it's actually invoked in the UI thread, you've got a different value of i, which may be null. Try this:

for (int i = 0; i < frmChildren.Length; i++)
{
    ChildGUI control = frmChildren[i];
    if (control != null)
    {
        control.BeginInvoke(new EventHandler(delegate
        {
            control.Close();
        }));
    }
}             
frmChildren = new ChildGUI[20];

See Eric Lippert's blog post for why introducing a new variable within the loop fixes the problem.

EDIT: If you want to use a foreach loop, it would look like this:

foreach (ChildGUI control in frmChildren)
{
    // Create a "new" variable to be captured
    ChildGUI copy = control;
    if (copy != null)
    {
        copy.BeginInvoke(new EventHandler(delegate
        {
            copy.Close();
        }));
    }
}             
frmChildren = new ChildGUI[20];

Just as an aside, you can use the fact that you just want to call a void method to make the code slightly simpler. As this no longer uses an anonymous method, you can make do away with the "inner" variable:

foreach (ChildGUI control in frmChildren)
{
    if (control != null)
    {
        control.BeginInvoke(new MethodInvoker(control.Close));
    }
}             
frmChildren = new ChildGUI[20];
Jon Skeet
I've tried the same code stated above with a foreach. How would a foreach be different?ODDLY your solution works! +1
Lily
Thanks for the link explaining why this works the way it does. If you didn't care about closing the forms asynchronously, I think you could replace BeginInvoke with Invoke instead of introducing another variable. Since Invoke is synchronous, the value of the iteration variable `i` will not have changed yet.
CodeSavvyGeek
@Lily: With a foreach loop, you'd still need to introduce an extra variable *inside the loop*. That's the important bit.
Jon Skeet