0

I am Iterating through a Dictionary and I want to trying to remove certain un required keys from the Dictionary .

My code is as follows

foreach (KeyValuePair<string, string> stackOfItems in ItemStack)
            {
                string Name = stackOfItems .Key;
                string Price= stackOfItems .Value;

                if (listOfSelectedOldItems.Contains(Name))
                {
                    ItemStack.Remove(Name);
                }

            }

All I am trying to do is removing a Dictionary entry (key and value) if a certain Key is in a List . The full error :- System.InvalidOperationException: 'Collection was modified; enumeration operation may not execute.' in dictionary

Can anyone please help me with this

Devss
  • 67
  • 7

1 Answers1

3

It's because foreach uses an enumerator and it becomes invalid when you remove items.

You can build a list of items to remove and do the actual removal in a second step.

var keysToRemove = new List<string>();
foreach (var stackOfItems in ItemStack)
{
    string Name = stackOfItems.Key;
    string Price= stackOfItems.Value;

    if (listOfSelectedOldItems.Contains(Name))
    {
        keysToRemove.Add(Name);
    }
}

foreach (var key in keysToRemove)
{
    ItemStack.Remove(key);
}

Update

As canton7 said, you can simply just try to remove all items that are in listOfSelectedOldItems.

foreach (var key in listOfSelectedOldItems)
{
    ItemStack.Remove(key);
}
jgauffin
  • 99,844
  • 45
  • 235
  • 372
  • 2
    As @canton7 already notices under the question, `listOfSelectedOldItems` is ... a list of keys to remove. – CodeCaster Jan 22 '20 at 13:11
  • Yeah, the work to test each of the `ItemStack` entries against `listOfSelectedOldItems` is, as far as I can see, entirely pointless (unless `listOfSelctedOldItems` is huge,`ItemStack` is tiny, and `listOfSelectedOldItems.Contains` is O(1), which I see no indication of) – canton7 Jan 22 '20 at 13:28
  • `listOfSelectedOldItems` might contain all items that are ***allowed*** to get removed, while `ItemStack` contains a subset. So yes, it can be pointless, but it can also be perfectly valid. – jgauffin Jan 22 '20 at 13:45
  • Indeed. However, `ItemStack.Remove` is O(1), even if `ItemStack` doesn't contain the element to remove. Given the naming of `listOfSelectedOldItems`, the `.Contains` call is likely to be O(n). Given these, I can't see a case where following your answer is quicker than just calling `ItemStack.Remove` for every item in `listOfSelectedOldItems`. – canton7 Jan 22 '20 at 13:50
  • Ahh. ok. didn't think of it in that way. That's perfectly true if having the fastest code is most important. – jgauffin Jan 22 '20 at 13:59