CODEWITHSUNDEEP
c#winforms
user3755946
user3755946

Reputation: 804

Foreach loop for disposing controls skipping iterations

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... enter image description here

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

enter image description here

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.

Upvotes: 6

Views: 1148

Answers (5)

toadflakz
toadflakz

Reputation: 7934

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());

Upvotes: 2

Steve
Steve

Reputation: 216342

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.

Upvotes: 8

MobileX
MobileX

Reputation: 419

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(); }

Upvotes: 0

Ilia Maskov
Ilia Maskov

Reputation: 1898

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);
}

Upvotes: 1

Khodaie
Khodaie

Reputation: 376

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())
{
    ...
}

Upvotes: 2

Related Questions