75

I need to remove multiple items from a Dictionary. A simple way to do that is as follows :

  List<string> keystoremove= new List<string>();
  foreach (KeyValuePair<string,object> k in MyCollection)
     if (k.Value.Member==foo)
        keystoremove.Add(k.Key);
  foreach (string s in keystoremove)
        MyCollection.Remove(s);

The reason why I can't directly Remove the items in the foreach block is that this would throw an Exception ("Collection was modified...")

I'd like to do the following :

 MyCollection.RemoveAll(x =>x.Member==foo)

But the Dictionary<> class doesn't expose a RemoveAll(Predicate<> Match) method, like the List<> Class does.

What's the best way (both performance wise and elegant wise) to do that?

Ruben Bartelink
  • 59,778
  • 26
  • 187
  • 249
Brann
  • 31,689
  • 32
  • 113
  • 162

9 Answers9

105

Here's an alternate way

foreach ( var s in MyCollection.Where(kv => kv.Value.Member == foo).ToList() ) {
  MyCollection.Remove(s.Key);
}

Pushing the code into a list directly allows you to avoid the "removing while enumerating" problem. The .ToList() will force the enumeration before the foreach really starts.

n00bmind
  • 346
  • 4
  • 11
JaredPar
  • 733,204
  • 149
  • 1,241
  • 1,454
  • 1
    Good answer, but I don't think the ToList() is required. Is it? – Jim Mischel Jan 22 '09 at 14:09
  • Good answer, but If I'm not mistaking, s is an instance of the Value type, so the ending s.key won't compile, or will it? – Brann Jan 22 '09 at 14:13
  • I don't think I like this solution too much because of the ".ToList()". Its there for a purpose, but its not self-evident what its purpose is until you remove .ToList() and observe the error yourself. I wouldn't recommend this code when there are more readable alternatives available. – Juliet Jan 22 '09 at 14:27
  • @JaredPar, please fix your answer, it is funny to see that most voted answer won't even compile. Also it is not necessary to query over values, you can get keys directly – aku Jan 22 '09 at 14:59
  • I'm waiting for someone with editing rights to fix this before I mark this post as the accepted answer. – Brann Jan 22 '09 at 15:28
  • 15
    @Jim, the .ToList is absolutely necessary here. The foreach body modifies the underlying collection. Without the .ToList() the Where clause will be operating against a modified collection. Using .ToList() forces the query to complete before any remove occurs – JaredPar Jan 22 '09 at 15:32
  • @aku, fixed the typo. I originally coded it a different way and then later came back and wanted to use KeyValuePairs. I forgot to update the selection. – JaredPar Jan 22 '09 at 15:32
  • @Princess, this unfortunately is just life with the implementation of IEnumerable. Yes it's a bit awkward the first time you see it but once you hit the pattern once it's not as bad. – JaredPar Jan 22 '09 at 15:33
  • I like it. I threw it into my set of Dictionary extensions. `public static T RemoveUnwanted(this T me, params V[] values) where T : IDictionary, new() { foreach (V v in values) { foreach (KeyValuePair kvp in me.Where(p => p.Value.Equals(v)).ToList()) { me.Remove(kvp.Key); } } return me; }` Use it like `myDict = myDict.RemoveUnwanted(myObj1, myObj2, ... myObjn);` – Machtyn Jul 20 '15 at 02:03
  • 2
    Would ToArray( ) be lighter than ToList( )? – Brain2000 Apr 01 '21 at 18:07
  • NOTE as of .NET Core 3, [this other answer is the solution - TL;DR just call `.Remove`, it no longer invalidates the iterator](https://stackoverflow.com/a/72415387/11635) – Ruben Bartelink Jul 28 '22 at 07:52
28

you can create an extension method:

public static class DictionaryExtensions
{
    public static void RemoveAll<TKey, TValue>(this IDictionary<TKey, TValue> dict, 
        Func<TValue, bool> predicate)
    {
        var keys = dict.Keys.Where(k => predicate(dict[k])).ToList();
        foreach (var key in keys)
        {
            dict.Remove(key);
        }
    }
}

...

dictionary.RemoveAll(x => x.Member == foo);
Community
  • 1
  • 1
aku
  • 122,288
  • 32
  • 173
  • 203
  • Works a treat. Thanks! – Nick Allan Feb 08 '19 at 15:01
  • If you want the value for the `predicate`, then why are you enumerating the keys, and you don't enumerate the `dict` directly in order to get the key-value pairs? By querying the dictionary for each key, you are losing some efficiency for no apparent reason. – Theodor Zoulias Feb 17 '21 at 06:57
15

Instead of removing, just do the inverse. Create a new dictionary from the old one containing only the elements you are interested in.

public Dictionary<T, U> NewDictionaryFiltered<T, U>
(
  Dictionary<T, U> source,
  Func<T, U, bool> filter
)
{
return source
  .Where(x => filter(x.Key, x.Value))
  .ToDictionary(x => x.Key, x => x.Value);
}
Amy B
  • 108,202
  • 21
  • 135
  • 185
11

Modified version of Aku's extension method solution. Main difference is that it allows the predicate to use the dictionary key. A minor difference is that it extends IDictionary rather than Dictionary.

public static class DictionaryExtensions
{
    public static void RemoveAll<TKey, TValue>(this IDictionary<TKey, TValue> dic,
        Func<TKey, TValue, bool> predicate)
    {
        var keys = dic.Keys.Where(k => predicate(k, dic[k])).ToList();
        foreach (var key in keys)
        {
            dic.Remove(key);
        }
    }
}

. . .

dictionary.RemoveAll((k,v) => v.Member == foo);
wimh
  • 15,072
  • 6
  • 47
  • 98
Jerome
  • 2,059
  • 1
  • 16
  • 19
  • 1
    I rolled back a change by Community because it did not include `ToList()` causing the "removing while enumerating" problem. – wimh Jun 20 '14 at 12:44
  • Thanks for the inspiration Jerome. In addition to this and @Aku's answer I created extension overloads for `Func` and `Func>` that all work along side each other (except if `TKey` and `TValue` are of the same type when the compiler obviously can't choose between the `Func` or the `Func`). If someone interested can't figure out how to implement them, ping me here and I'll post them. :-) – Ulf Åkerstedt Nov 18 '15 at 09:01
4

Starting from the .NET 3.0, it's now allowed to remove items from a Dictionary<TKey,TValue> while enumerating it. According to the documentation:

.NET Core 3.0+ only: The only mutating methods which do not invalidate enumerators are Remove and Clear.

Here is the GitHub issue where this change was proposed and approved: Allow Dictionary<K,V>.Remove during enumeration

So the RemoveAll extension method can be implemented simply like this:

/// <remarks>.NET Core 3.0+ only.</remarks>
public static void RemoveAll<TKey, TValue>(this Dictionary<TKey, TValue> source,
    Predicate<KeyValuePair<TKey, TValue>> predicate)
{
    foreach (var pair in source)
        if (predicate(pair))
            source.Remove(pair.Key);
}

Usage example:

myDictionary.RemoveAll(e => e.Value.Member == foo);
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
  • 1
    Great answer! I didn't know `Remove` could be used while enumerating now. I ran a quick benchmark and found this approach almost twice as fast as using LINQ when removing 50% of the items in a dictionary. – matisse Dec 11 '22 at 06:39
1

The fastest way to remove would be either:

public static void RemoveAll<TKey, TValue>(this IDictionary<TKey, TValue> idict, Func<KeyValuePair<TKey, TValue>, bool> predicate)
    {
        foreach (var kvp in idict.Where(predicate).ToList())
        {
            idict.Remove(kvp.Key);
        }
    }

or

public static void RemoveAll<T>(this ICollection<T> icollection, Predicate<T> predicate)
{
    var nonMatchingItems = new List<T>();

    // Move all the items that do not match to another collection.
    foreach (var item in icollection) 
    {
        if (!predicate(item))
        {
            nonMatchingItems.Add(item);
        }
    }

    // Clear the collection and then copy back the non-matched items.
    icollection.Clear();
    foreach (var item in nonMatchingItems)
    {
        icollection.Add(item);
    }
}

depending on whether you have more cases of predicate returning true or not. Both are O(N) in nature, but 1st approach will be faster if you have very less cases of "removal/lookup", and the second one faster if items in collection matches the condition majority of the times.

nawfal
  • 70,104
  • 56
  • 326
  • 368
0

Can you just change your loop to use an index (i.e. FOR instead of FOREACH)? You'd have to loop backwards, of course, i.e. count-1 down to zero.

Geoff
  • 8,551
  • 1
  • 43
  • 50
  • 1
    You can't iterate through a Dictionary with FOR. – Brann Jan 22 '09 at 14:14
  • 2
    Sorry, I thought the extension .ElementAt(index) would allow this, at least in .net 3.5. – Geoff Jan 22 '09 at 15:44
  • @brann oh, you can using linq. for (int index = 0; index < MyCollection.Count; index++) { var kvp = MyCollection.ElementAt(index); var kvp = item.Key; var kvp = item.Value; } – Offler Jun 12 '13 at 09:06
  • 1
    @Geoff: Suppose the dictionary contains three keys: "Moe", "Larry", and "Curly". }; one wants to delete all keys not starting with "C". The first call to `ElementAt(2)` will enumerate all three items and return Curly, who should not be deleted. Then `ElementAt(1)` will enumerate two items, and return Larry. Deleting Larry may arbitrarily resequence the items, so `ElementAt(0)` might return Moe or Curly. If it happens to return Curly, then Moe will not end up getting processed. `ElementAt` may be legal, but that doesn't mean it will work usefully. – supercat Sep 20 '14 at 22:24
0

Instead of removing just do the inverse (create a new dictionary from the old one containing only the elements you are interested in) and let the garbage collector take care of the old dictionary:

var newDictionary = oldDictionary.Where(x => x.Value != foo);
Ian Kemp
  • 28,293
  • 19
  • 112
  • 138
Darin Dimitrov
  • 1,023,142
  • 271
  • 3,287
  • 2,928
-2

It's simple using LINQ. Just do the following :)

MyCollection = MyCollection.Where(mc => !keystoremove.Contains(mc.Key))
.ToDictionary(d => d.Key, d => d.Value);
Siyavash Hamdi
  • 2,764
  • 2
  • 21
  • 32