CODEWITHSUNDEEP
c#winforms
Marcelo Camargo
Marcelo Camargo

Reputation: 2298

Remove item from Menu Strip

I have a Menu Strip in Windows Forms with the following structure, named mstMain:

And I'm doing a for loop to remove all items in Programs (named mnuPrograms) where the number of items is == 0. My attempt is:

  for (int n = 0; n < mnuPrograms.MenuItems.Count; n++)
                        if (mnuPrograms.MenuItems[n].MenuItems.Count == 0)
                            mnuPrograms.MenuItems.Remove(mnuPrograms.MenuItems[n]);

And it doesn't remove the correct items. I have used foreach too:

foreach (
     MenuItem item in
       mnuPrograms.MenuItems.Cast<MenuItem>().Where(item => item.MenuItems.Count == 0))
                        mnuPrograms.MenuItems.Remove(item);

But in this case I receive a InvalidOperationException. How can I perform this?

Upvotes: 0

Views: 1273

Answers (2)

Dude Pascalou
Dude Pascalou

Reputation: 3179

You are modifiying the list while you are looping through it.

  • In the for clause, the indexes change when you remove an item. Try looping through the list from the end to the start.
  • In the foreach clause, modifying a list while enumerating it ends with an InvalidOperationException.

And pid is right, you may disable an item rather than delete it.

Upvotes: 3

pid
pid

Reputation: 11607

On a Usability point of view, you might prefer to disable the items. Users want to see that they cannot do something. By removing them they may get confused. Once you disable the items your problem might already be solved.

EDIT:

You also have a false assumption that denotes a deeper misunderstanding. You are doing two things:

  • iterating over a set
  • modifying the set

You shouldn't do both because termination conditions become unpredictable (NP-incomplete under circumstances) if you don't have an invariant set over which you enumerate. That's why a foreach will throw an exception if you modify the enumerated set inside the block.

To avoid this you should increment only when you don't remove, but at that point a while() is more elegant, much like this:

while (i < list.Count)
{
    // do stuff with list[i]

    if (removal_condition) // only do one of these, never both
    {
        list.RemoveAt(i);
    }
    else
    {
        i++;
    }
}

Upvotes: 2

Related Questions