Method to remove text boxes from panel

Question

I have a method in a windows form application that tries to remove 2 text boxes from a panel.

In the method, I loop through all the controls in the panel. There are always supposed to be 2 panels removed and added together, but when removing, it randomly removes 1 or 2 containers when I press the button.

Here is the code to remove the textboxes:

private void removeRows()
        {
            string descName = "Desc" + (textBoxCounter - 1).ToString();
            string costName = "Cost" + (textBoxCounter - 1).ToString();

            if (textBoxCounter >= 0)
            {
                foreach (Control c in costItems.Controls)
                {
                    if (c.Name == descName)
                    {
                        // Remove the control from the panel and dispose of it
                        panel.Controls.Remove(c);
                        c.Dispose();

                    }
                    if(c.Name == costName)
                    {
                        // Remove the control from the panel and dispose of it
                        panel.Controls.Remove(c);
                        c.Dispose();
                    }
                }

                // Decrement the counter
                // This happens only once since two controls need to be removed
                if (textBoxCounter == 0)
                    textBoxCounter = 0;
                else
                    textBoxCounter--;
            }
            else
                MessageBox.Show("There are no more rows to remove", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);

            testlabel1.Text = textBoxCounter.ToString();
            testlabel2.Text = panel.Controls.Count.ToString();
        }

Here is the code to add a button:

private void addRows(string desc, string cost)
    {
        if (textBoxCounter >= maxExpenses)
        {
            MessageBox.Show("Maximum number of expenses entered", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
        }
        else
        {
            TextBox Desc = new TextBox();
            TextBox Cost = new TextBox();

            // Give the text boxes names
            Desc.Name = "Desc" + textBoxCounter.ToString();
            Cost.Name = "Cost" + textBoxCounter.ToString();

            // Format the text boxes
            Desc.Width = panel.Width / 2;
            Cost.Width = panel.Width / 4;

            // Add the items to the costItems panel
            panel.Controls.Add(expenseDesc);
            panel.Controls.Add(expenseCost);

            // Add the items to the expenses dictionary
            panel.Add(Desc, Cost);

            // Increment the text box counter variable
            textBoxCounter++;
            testlabel1.Text = textBoxCounter.ToString();
            testlabel2.Text = costItems.Controls.Count.ToString();
        }
    }

Some info to know. There will always be 2 textboxes added and removed, they relate to each other. The textBoxCounter is initialized to 0, so the first two boxe names will be "Desc0" and "Cost0".

When I press the button to remove rows the first time, one text box is removed, and then if i press it again it might remove 2, it might only remove 1.

I tried debugging and I noticed that the foreach loop that iterates over all the controls in the panel seems to loop one time short of the full number of controls.

Any help with my code would be great.

Answer 1

Your code has two problems: you are disposing something you cannot dispose and you are iterating through a collection (which you are modifying) in the wrong way. You can delete all the Controls by doing:

panel.Controls.Clear();

Or iterating backwards by relying on the indices:

for (int i = panel.Controls.Count - 1; i >= 0; i--)
{
    panel.Controls.RemoveAt(i);
}

Regarding the Dispose, you can use it if you wish but don't need to use Remove:

for (int i = panel.Controls.Count - 1; i >= 0; i--)
{
    panel.Controls[i].Dispose();
}

PS: I asked something identical to this and got -6. One of the reasons for maintaining this question was precisely being helpful to others (I saw the code you are using to delete controls in internet and I knew that quite a few people were using it). Pretty ironical, indeed.

Answer 2

Your problem is caused by the foreach, modifying the collection in foreach may cause some unexpected behavior. You just want to remove the TextBoxes with names being known beforehand, so why not using the method ControlCollection.RemoveByKey?

If you want to remove the last added textBoxes (Desc... and Cost...) do this:

panel.Controls.RemoveByKey(descName);
panel.Controls.RemoveByKey(costName);

If you want to remove all the added textBoxes (suppose you have other kinds of TextBoxes, otherwise we can use a little LINQ to remove all the textboxes easily):

for(int i = 0; i < textBoxCounter; i++){
   panel.Controls.RemoveByKey("Desc" + i);
   panel.Controls.RemoveByKey("Cost" + i);
}