Help with converting foreach loop to while loop in c#

Refresh

December 2018

Views

1.9k time

1

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

5 answers

0

Вместо foreachиспользования обратного for-loop:

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

Fun с 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);
    }
}
0

Попробуйте что-то вроде этого:

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

На мой взгляд, способ сделать код работы является:
1. Создать экземпляр типа favoritesToolStripMenuItem.DropDownItemsколлекции.
2. В foreachцикле, добавить все элементы, вы не хотите , чтобы быть удалены, в этой коллекции.
3. Убедитесь , favoritesToolStripMenuItem.DropDownItemsчтобы указать на новую коллекцию. Или ясно favoritesToolStripMenuItem.DropDownItemsи загружать элементы из новой коллекции к нему.

Надеюсь это поможет

3

Вам не нужен foreachцикл - просто использовать обычный цикл , но идти в обратном направлении, начать с конца и перейти к началу.

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

это @ код Kurresmack в переставить, я просто закодирован его прямо здесь на странице поэтому извините любую небольшую ошибку синтаксиса или что-нибудь очевидное я упускать из вида (отказ от ответственности: это образец !!)

Вы все еще можете рассматривать favoritesToolStripMenuItem.DropDownItemsкак коллекцию , как вы были, но вы не должны перечислить над ним с помощью foreach. Это сокращает на несколько строк кода, и это работает , потому что вы переборе его в обратном порядке, вы не получите индекс из исключения границ.