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.
精彩评论