Foreach loop for disposing controls skipping iterations

Question

Code to create textboxes...

private void btnAddIncrement_Click(object sender, EventArgs e)
{              
    SmartTextBox dynamictextbox = new SmartTextBox();

        dynamictextbox.BackColor = Color.Bisque;
        dynamictextbox.Width = this.tbWidth;
        dynamictextbox.Left = (sender as Button).Right + this.lastLeft;
    dynamictextbox.K = "Test";

    this.lastLeft = this.lastLeft + this.tbWidth;
    dynamictextbox.Top = btnAddStart.Top;
    this.Controls.Add(dynamictextbox);              
}

Code for to remove all text boxes.

foreach (Control c in this.Controls)
{

    if (c.GetType() == typeof(BnBCalculator.SmartTextBox))
    {
        count++;
        //MessageBox.Show((c as SmartTextBox).K.ToString());
        c.Dispose();
    }
   // else { MessageBox.Show("not txtbox"); }

}

When I click the btnAddIncrement I get the following as expected...

But when I click reset it misses every second textbox. See below...

No idea what's going on here but this is the same no matter how may text boxes I add. It always misses every second box.

Possible duplicate of [foreach control c# skipping controls](https://stackoverflow.com/questions/17627038/foreach-control-c-sharp-skipping-controls) — Lance U. Matthews, Oct 02 '18 at 22:49

score 8 · Accepted Answer · edited May 23 '17 at 12:15

You should use a reverse standard for loop to dispose the SmartTextBoxes from its container

for(int x = this.Controls.Count - 1; x >= 0; x--)
{
    BnBCalculator.SmartTextBox c = this.Controls[x] as BnBCalculator.SmartTextBox;
    if (c != null)
    {
        count++;
        c.Dispose();
    }
}

According to this question/answer you don't need to remove them from the container and of course this avoids two loops (explicit or implicit). Also in the accepted answer you could see the reason why your code jumps a control every two.

if (parent != null) 
{ 
    parent.Controls.Remove(this); 
}

The control that you want to dispose is removed from the collection that you are iterating over. (Not clear why this doesn't throw the standard exception).

Instead looping with a simple for in reverse avoid any problem in the ordered access to the controls to dispose.

toadflakz · Answer 2 · 2016-01-29T12:07:52.013

2

Your removal code is incorrect as you are modifying the Controls collection by calling Dispose() which is why you get the skipping of controls.

Easiest option to remove those of a specific type is to do the following:

var smartTbs = this.Controls.OfType<BnBCalculator.SmartTextBox>().ToList();
smartTbs.ForEach(x => x.Dispose());

edited Jan 29 '16 at 12:07

answered Jan 29 '16 at 11:56

toadflakz

7,764
1
27
40

score 2 · Answer 3 · answered Jan 29 '16 at 11:56

When you remove an item form this.Controls the collection is modified and so the next item is not what you expect. Yo should copy the this.Controls to a new list. For example you can use ToArray to make a copy of this.Controls

foreach (Control c in this.Controls.ToArray())
{
    ...
}

score 1 · Answer 4 · answered Jan 29 '16 at 11:56

You have to remove controls from Form.Controls at first and then dispose it.

var controlsToRemove = new List<Control>();
foreach (Control c in this.Controls)
{
    if (c is BnBCalculator.SmartTextBox) 
        controlsToRemove.Add(c);
}

foreach (Control c in controlsToRemove)
{
    Controls.Remove(c);
}

score 0 · Answer 5 · answered Jan 29 '16 at 11:58

0

Try to select all the SmartTextBox controls first and dispose them on another loop. Pseudocode:

    SmartTextBoxes = Select From this.Controls Where (c.GetType() == typeof(BnBCalculator.SmartTextBox));
    foreach(stb in SmartTextBoxes) { stb.Dispose(); }

answered Jan 29 '16 at 11:58

MobileX

409
4
13

1

You could just use LINQ's `OfType()` method. – toadflakz Jan 29 '16 at 12:02
Exactly. But its a bit difficult to write correct LINQ expression outside IDE. So I've just written the 'pseudocode' keeping in mind LINQ. – MobileX Jan 29 '16 at 12:19

Foreach loop for disposing controls skipping iterations

5 Answers5

Linked