开发者

Code Analysis CA2000: Is this code okay?

Have a look at this code, I believe it solved CA2000 but I want to make sure I'm not overlooking something. Basically this code loads a new Control based on what is selected in a TreeView. That Control is then displayed and is visible/usable until another Node in the TreeView is selected.

private void Something(object sender, TreeViewEventArgs e)
{
    ProjectTreeNode node = (e.Node as ProjectTreeNode);

    foreach (Control c in optionsPlaceholderPanel.Controls)
        c.Dispose();

    optionsPlaceholderPanel.Controls.Clear();

    if (node != null)
    {
        //ProjectOptions inherits from Control and is therefore IDisposable
        ProjectOptions options = new ProjectOptions(node.Project);

        ShowOptionsPanel(options);
    }
}

private void ShowOptionsPanel(Control control)
{
    optionsPlaceholderPanel.Controls.Add(control);
    control.Dock = DockStyle.Fill;
}

So basically the Control is in scope always, until a new Control is loaded in place of it. Whe开发者_StackOverflow中文版n I do that, I'm disposing the prior-loaded Control so I think it's safe to ignore CA2000 in this case. Also, when the Form finally closes and optionsPlaceholderPanel is disposed, this will also dispose the child controls, right?


foreach (Control c in optionsPlaceholderPanel.Controls)
    c.Dispose();

No, this code has a bug. Which in itself is triggered by a bug in the ControlCollection class. Your foreach loop is modifying the panel's Controls collection. This normally produces an InvalidOperationException, "Collection was modified, enumeration operation may not execute", but the class forgets to do this.

The Dispose() call on the control removes it from the collection. In effect, you'll only dispose every other control. This should have a side-effect, they remain visible on the panel. Ymmv. Fix:

        for (int ix = optionsPlaceholderPanel.Controls.Count - 1; ix >= 0; --ix)
            optionsPlaceholderPanel.Controls[ix].Dispose();

Or less efficient, although you'd never see the difference:

        while (optionsPlaceholderPanel.Controls.Count > 0)
             optionsPlaceholderPanel.Controls[0].Dispose();

Otherwise the code is okay, CA2000 tends to produce false warnings.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜