Help with converting foreach loop to while loop in c#

Question

I had learnt by reading your great answers here, that it is not good practice deleting items from within a foreach loop, as it is (and I quote) "Sawing off the branch you're sitting on".

My code currently removes the text from the dropdownlist, but the actual item remains (just without text displayed).

In other words, it isn't deleting, and probably can't because you can't delete from within a foreach loop.

After hours of trying I am unable to get my head around a way of doing it.

//For each checked box, run the delete code
for (int i = 0; i < this.organizeFav.CheckedItems.Count; i++)
{
    //this is the foreach loop
    foreach (ToolStripItem mItem in favoritesToolStripMenuItem.DropDownItems)
    {
        //This rules out seperators
        if (mItem is ToolStripMenuItem)
        {
            ToolStripMenuItem menuItem = mItem as ToolStripMenuItem;

            //This matches the dropdownitems text to the CheckedItems String
            if (((ToolStripMenuItem)mItem).Text.ToString() == organizeFav.CheckedItems[i].ToString())
            {
                //And deletes the item
                menuItem.DropDownItems.Remove(mItem);
            }
        }
    }
}

But it isn't deleting because it is within a foreach loop! I would greatly appreciate your help, and be truly amazed if anyone can get their head around this code :)

Kind Regards

Tanzelax · Accepted Answer · 2010-03-24 07:31:39Z

5

Fun with LINQ!

// Loop through the checked items, same as you did.
foreach (var checkedItem in this.organizeFav.CheckedItems)
{
    // Cast from IEnumerable to IEnumerable<T> so we can abuse LINQ
    var matches = favoritesToolStripMenuItem.DropDownItems.Cast<ToolStripItem>()
                  // Only items that the Text match
                  .Where(item => item.Text == checkedItem.Text)
                  // Don't match separators
                  .Where(item => item is ToolStripMenuItem)
                  // Select the keys for the later .Remove call
                  .Select(item => item.Name);

    // Loop through all matches        
    foreach (var key in matches)
    {
        // Remove them with the Remove(string key) overload.
        favoritesToolStripMenuItem.Remove(key);
    }
}

answered Mar 24, 2010 at 7:31

Tanzelax

5,7242 gold badges28 silver badges28 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

slugster Over a year ago

Yep, Linq would also be my preferred option, but i didn't want to scare the OP with its awesome power :)

slugster · Accepted Answer · 2010-03-24 07:10:43Z

You don't need a foreach loop - just use a regular loop but go in reverse, start at the end and go to the beginning.

//For each checked box, run the delete code
for (int i = 0; i < this.organizeFav.CheckedItems.Count; i++)
{
    //this *replaces* the foreach loop
    for(int j = favoritesToolStripMenuItem.DropDownItems.Count - 1; j >= 0; j--)
    {
        ToolStripMenuItem menuItem = favoritesToolStripMenuItem.DropDownItems[j] as ToolStripMenuItem; 

        //This rules out seperators
        if (menuItem != null)
        {
            //This matches the dropdownitems text to the CheckedItems String
            if (menuItem.Text.ToString() == organizeFav.CheckedItems[i].ToString())
            {
                favoritesToolStripMenuItem.DropDownItems.Remove(menuItem);
            }
        }
    }
 }

this was @Kurresmack's code rearranged, i just coded it directly here in the page so excuse any small syntax error or anything obvious i overlooked (disclaimer: it is a sample!!)

You can still treat favoritesToolStripMenuItem.DropDownItems as a collection like you were, but you don't have to enumerate over it using a foreach. This cuts down on a few lines of code, and it works because you are iterating it in reverse order, you will not get an index out of bounds exception.

Oskar Kjellin · Accepted Answer · 2010-03-24 06:50:13Z

Try something like this:

//For each checked box, run the delete code
for (int i = 0; i < this.organizeFav.CheckedItems.Count; i++)
        {
    List<ToolStripItem> toRemove = new List<ToolStripItem>();
//this is the foreach loop
            foreach (ToolStripItem mItem in favoritesToolStripMenuItem.DropDownItems)
            {

                //This rules out seperators
                if (mItem is ToolStripMenuItem)
                {
                    ToolStripMenuItem menuItem = mItem as ToolStripMenuItem;

             //This matches the dropdownitems text to the CheckedItems String
                    if (((ToolStripMenuItem)mItem).Text.ToString() == organizeFav.CheckedItems[i].ToString())
                    {
                        toRemove.Add(mItem);
                    }
                }
            }
        foreach(var item in toRemove)
        {
        favoritesToolStripMenuItem.DropDownItems.Remove(item);
        }
        }

Vitali Climenco · Accepted Answer · 2010-03-24 07:06:04Z

0

To my mind, the way to make the code work is:
1. Create an instance of the type the favoritesToolStripMenuItem.DropDownItems collection is.
2. In the foreach loop, add all items, you do not want to be removed, to that collection.
3. Make favoritesToolStripMenuItem.DropDownItems to point to the new collection. Or clear favoritesToolStripMenuItem.DropDownItems and load the items from the new collection to it.

Hope this helps

answered Mar 24, 2010 at 7:06

Vitali Climenco

1,3241 gold badge12 silver badges19 bronze badges

Comments

Oliver · Accepted Answer · 2010-03-24 07:22:09Z

0

Instead of a foreach use a reverse for-Loop:

for(int reverseIndex = myList.Count - 1; reverseIndex >= 0; reverseIndex--)
{
    var currentItem = myList[reverseIndex];
    if(MatchMyCondition(currentItem))
    {
        myList.Remove(currentItem);
    }
}

answered Mar 24, 2010 at 7:22

Oliver

45.4k8 gold badges101 silver badges164 bronze badges

Collectives™ on Stack Overflow

Help with converting foreach loop to while loop in c#

5 Answers 5

1 Comment

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

1 Comment

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related