views:

305

answers:

9

Here's the code I have:

private void ClearSearchResults()
    {
        foreach (Control X in panel1.Controls)
        {
            panel1.Controls.Remove(X);
        }
    }

The problem is, when I run this method, only a single item is deleted, then if I click on a button again so the method can run again, another is deleted.

If I have 10 control in my panel, I'd have to click the "Delete" button on my program many times for all the control to be deleted.

What can I do in this case?

+10  A: 

Does this work for you?

private void ClearSearchResults()
{
    panel1.Controls.Clear();
}

Edited to emphasize the CKret comment.

Austin Salonen
Yeah it works. Really wierd. Why doesn't the foreach statement do the trick though?
Sergio Tapia
+1 beat me to it...
Sani Huttunen
@Papuccino1, my answer might explain that.
Stan R.
As Star R said: It doesn't work with a foreach since you are changing the IEnumerable while you are iterating throught it. That is forbidden.
Sani Huttunen
@Papuccino1: Check out my answer for the typical workaround. This issue happens with nearly ANY collection and "foreach"
Reed Copsey
Just to be clear: Remove the foreach loop alltogether. I.e. do not put panel1.Controls.Clear(); inside the foreach loop.
Sani Huttunen
+8  A: 

I believe you are changing the IEnumareble when you remove an item from it while iterating it.

Try to use a simple for loop instead of a foreach.

Stan R.
This is the correct explanation. You cannot alter a collection while enumerating it.
sindre j
If removing all items then you should use a regular for loop and loop backwards. Otherwise you will remove every other item and then get an OutOfBounds exception.
Sani Huttunen
@CKret, yes Reed's example illustrates how to deal with it.
Stan R.
+2  A: 

Maybe this:

panel1.Controls.Clear()
Sani Huttunen
+1  A: 

As I don't know the kind of panel you use, you can generally call panel1.Controls.Clear

Johannes Rudolph
+11  A: 

You, in general, can't remove from a collection while iterating an enumerable generated from it. Instead of using foreach, the typical approach is to use a for loop working backwards:

private void ClearSearchResults()
{
    for(int i=panel1.Controls.Count-1;i>=0;--i) {
        panel1.Controls.RemoveAt(i);        
        // or
        // Control X = panel1.Controls[i];
        // panel1.Controls.Remove(X);
    }
}

However, in this case, just use clear:

panel1.Controls.Clear();
Reed Copsey
+1 this is what i was getting at, thanks for being less lazy than me Reed. :)
Stan R.
Hehehe, I didn't see yours when I started, or I probably wouldn't have bothered :)
Reed Copsey
+3  A: 

For a start you shouldn't edit the IEnumerable collection within a foreach loop. You should be using a for loop or a while.

I.e.

private void ClearSearchResults()
    {
        while (panel1.Controls.Count > 0)
        {
            panel1.Controls.RemoveAt(0);
        }
    }

or just use:

 panel1.Controls.Clear();
GenericTypeTea
this code wouldn't work, RemoveAt(0) ? you need an indexer
Stan R.
It will work. Since if there is atleast one element in the collection then there is always an element at index 0.
Sani Huttunen
@Stan R - it's a while loop, so it won't need an indexer
GenericTypeTea
@GenericTypeTea, oops I completely glossed over that. Im going to leave my comment there for sheer humiliation.. and +1 for you.
Stan R.
@Stan R, no worries chap, we all make mistakes. You should see some of mine.
GenericTypeTea
A: 
  private void ClearSearchResults()
        {
            foreach (Control X in panel1.Controls)
            {
                panel1.Controls.Remove(X);
            }
            if (panel1.Controls.Count > 0)
            { 
                ClearSearchResults();
            }
        }
Crash893
this is bad code, look at other explanations.
Stan R.
are you kidding?
Neil N
It works ( I saw the clear option after i posted)
Crash893
it works, by breaking...
Stan R.
I'm not saying this is a great answer but it does work.
Crash893
A: 

Hi,

Actually you can't use Remove because it breaks the iterator so simple solution would be something like this :

var controls = in c from panel1.Controls select c;
foreach(Controls _control in controls)
{
   panel1.Controls.Remove(_control);
}

but of course you don't want to stick to Loop then go ahead and use panel1.Controls.Clear()

Braveyard
Of course this would not work as when you remove a control you change the existing controls index. You are also looping too much. Imagine having 2 controls your loop says start at 0 and end at index = 2. The minute you remove one control you actually have changed the index of all current controls. The solution is to simply loop for the number of controls but remove at index 0.int n; n = panel1.Controls.Count-1; for (int i = 0; i <= n; i++ ) { Control c = panel1.Controls[0]; panel1.Controls.Remove(c); }
JonH
@JonH, yes you are right, I edited my post.
Braveyard
A: 
            int n;
            n = panel1.Controls.Count-1;

            for (int i = 0; i <= n; i++ )
            {
                Control c = panel1.Controls[0];
                panel1.Controls.Remove(c);
            }
JonH