Modify a dictionary which I am iterating through

Question

foreach(BruteforceEntry be in Entries.Values)
{
    if (be.AddedTimeRemove <= now)
        Entries.Remove(be.IPAddress);
    else if (be.Unbantime <= now && be.Unbantime.Day == DateTime.Now.Day)
        Entries.Remove(be.IPAddress);
}

An exception was thrown:

Collection was modified; enumeration operation may not execute.

For some reason, it is not any more.

I know you cannot remove something, while iterating through it this way. My question is: How do I solve it?

Answer 1

You can't modify a collection you're iterating over. In this case, a much better solution would be to create a list of entries to remove by iterating over the dictionary, and then iterate over that list, removing the entries from the dictionary:

List<string> removals = new List<string>();                    
DateTime now = DateTime.Now;
foreach(BruteforceEntry be in Entries.Values)
{
    if (be.AddedTimeRemove <= now ||
        (be.Unbantime <= now && be.Unbantime.Day == DateTime.Now.Day))
    {
        removals.Add(be.IPAddress);
    }
}
foreach (string address in removals)
{
    Entries.Remove(address);
}

Note that if you're using .NET 3.5, you could use a LINQ query to express the first part:

List<string> removals = (from be in Entries.Values
                         where be.AddedTimeRemove <= now ||
                               (be.Unbantime <= now && 
                                be.Unbantime.Day == DateTime.Now.Day)
                         select be.IPAddress).ToList();

Answer 2

Simply put: you can't remove an entry from a collection while you are iterating over it.

One possible workaround is to create a shallow copy of the collection (e.g using ToList) and iterate over that :

foreach(BruteforceEntry be in Entries.Values.ToList())
{
    // modify the original collection
}

Answer 3

This (in my opinion) is the easiest way:

Dictionary<String, String> A = new Dictionary<string, string>(); //Example Dictionary
A.Add("A", "A"); //Example Values
A.Add("B", "B");
A.Add("C", "C");

for (int i = A.Count - 1; i >= 0; i--) //Loop backwards so you can remove elements.
{
     KeyValuePair<String, String> KeyValue = A.ElementAt(i); //Get current Element.
     if (KeyValue.Value == "B") A.Remove(KeyValue.Key);
}

In your case:

for (int i = Entries.Count - 1; i >= 0; i--)
{
    KeyValuePair<String, BruteforceEntry> KeyValue = Entries.ElementAt(i);
    if (KeyValue.Value.AddedTimeRemove <= now)
         Entries.Remove(KeyValue.Key);
    else if (KeyValue.Value.Unbantime <= now && KeyValue.Value.Unbantime.Day == DateTime.Now.Day)
         Entries.Remove(KeyValue.Key);
}

Answer 4

You can change foreach(BruteforceEntry be in Entries.Values) to foreach(BruteforceEntry be in new List<BruteforceEntry>(Entries.Values))

That way you don't modify your collection, but rather a copy of it.

Answer 5

It looks like your question is about why an exception is not being thrown where it used to be. As the other answers state, you generally cannot change a collection through which you are iterating, but you are iterating over the Values collection, which, I believe, is a copy of the values in the dictionary, rather than referring to the main dictionary collection itself. So it no longer has the problem of iterating and modifying the same thing.

Answer 6

You can't modify the collection when iterating over it with foreach. Instead, iterate over it with a for loop.

Answer 7

Boiling down your problem...

foreach(... ... in Entries.Values)
{
        Entries.Remove(...);

}

As the others have said you are modifying the iterator while iterating.

You can, as @David said, use a for loop instead, but make sure to start at the end (reverse iteration).

Answer 8

I tried another approach that works great for smaller dictionaries.

The idea is to just iterate the dictionary from start until everything is removed. Because the enumerator is a struct, it is allocation free in .NET Core. Of course this method should not be used for large dictionaries.

while (true)
{
    var isRemoved = false;

    foreach (var kvp in dict)
    {
        if (kvp.Key == "15" || kvp.Key == "20")
        {
            source.Remove(kvp.Key);
            isRemoved = true;
            break;
        }
    }

    if (!isRemoved)
    {
        break;
    }
}

I tested a few alternatives:

[SimpleJob]
[MemoryDiagnoser]
public class DictionaryRemove
{
    private Dictionary<string, string> source;

    [IterationSetup]
    public void Setuo()
    {
        source = new Dictionary<string, string>();

        for (var i = 0; i < 20; i++)
        {
            source.Add($"{i}", $"{i}");
        }
    }

    [Benchmark]
    public void Remove_Linq_Keys()
    {
        foreach (var key in source.Keys.Where(x => x == "15" || x == "20").ToList())
        {
            source.Remove(key);
        }
    }

    [Benchmark]
    public void Remove_Linq_Normal()
    {
        foreach (var kvp in source.Where(x => x.Key == "15" || x.Key == "20").ToList())
        {
            source.Remove(kvp.Value);
        }
    }

    [Benchmark]
    public void Remove_Plain()
    {
        while (true)
        {
            var isRemoved = false;

            foreach (var kvp in source)
            {
                if (kvp.Key == "15" || kvp.Key == "20")
                {
                    source.Remove(kvp.Key);
                    isRemoved = true;
                    break;
                }
            }

            if (!isRemoved)
            {
                break;
            }
        }
    }
}

And the results

|             Method |      Mean |     Error |    StdDev |    Median | Allocated |
|------------------- |----------:|----------:|----------:|----------:|----------:|
|   Remove_Linq_Keys | 16.680 us | 1.2932 us | 3.7519 us | 16.200 us |     688 B |
| Remove_Linq_Normal |  6.755 us | 0.4254 us | 1.1928 us |  6.200 us |     720 B |
|       Remove_Plain |  1.936 us | 0.1540 us | 0.4319 us |  1.800 us |     480 B |

If you know that your dictionary is relatively small, it might be a good alternative.