Removing elements from a Dictionary

ForEach

Kirill Osenkov — Sun, 01 Feb 2009 10:42:21 GMT

In my recent post about coding styles one particular thing provoked the majority of feedback and discussions:

re: Removing elements from a Dictionary

Pilchie (Kevin Pilch-Bisson) — Fri, 31 Oct 2008 03:22:17 GMT

Uh-oh!

You caught me making perf comments based on pure reasoning instead of empirical evidence.

I didn't actually measure. I'd love to hear about the results of your measurements though...

re: Removing elements from a Dictionary

Rik Hemsley — Fri, 31 Oct 2008 02:39:38 GMT

Kevin, did you measure your ToArray() solution vs. my solution using a copy of the dictionary? I haven't profiled for memory usage, but a rather unscientific test using System.Diagnostics.Stopwatch shows the dictionary copy solution is clearly faster. I'd be interested to see what happens with RAM, but I'll have to wait until tomorrow to use a proper profiler.

re: Removing elements from a Dictionary

Pilchie (Kevin Pilch-Bisson) — Thu, 30 Oct 2008 17:14:57 GMT

Rik, Your solution also works, but the price you pay is the space and time overhead of creating a completly new dictionary that contains all of the original elements. In a large collection, you may not want to pay that price, assuming that the average case is that you're only actually removing half of them. You also don't need the second collection to actually be a dictionary, since you aren't making any use of the lookup functionality; you're just enumerating the contents.

re: Removing elements from a Dictionary

Pilchie (Kevin Pilch-Bisson) — Thu, 30 Oct 2008 17:13:52 GMT

Kirill, that _works_, but it loses the ability to do the delayed execution that Linq gets you. It also means that you can't chain multiple "ForEach" calls together to do different things.

Without delayed execution, you can't "ForEach/Apply" on an infinite collection, and not having the ability to chain means that you have lump all of your side-effects together.

re: Removing elements from a Dictionary

Rik Hemsley — Thu, 30 Oct 2008 12:34:09 GMT

public static Dictionary<Tkey, Tvalue> DeleteWhere<Tkey, Tvalue>(this Dictionary<Tkey, Tvalue> collection, Func<Tkey, Tvalue, bool> shouldDelete)

{

foreach (var keyValuePair in new Dictionary<Tkey, Tvalue>(collection))

{

if (shouldDelete(keyValuePair.Key, keyValuePair.Value))

collection.Remove(keyValuePair.Key);

}

return collection;

}

re: Removing elements from a Dictionary

Kirill Osenkov - MSFT — Thu, 30 Oct 2008 11:10:38 GMT

And how about:

using System;

using System.Collections.Generic;

using System.Linq;

static class Program

{

static void Main(string[] args)

{

Dictionary<string, int> d = new Dictionary<string, int>()

{

{"one", 1},

{"two", 2}

};

d.Where(pair => pair.Value < 2).ToArray().ForEach(pair => d.Remove(pair.Key));

}

static void ForEach<T>(this IEnumerable<T> list, Action<T> action)

{

foreach (var item in list)

{

action(item);

}

re: Removing elements from a Dictionary

Pilchie (Kevin Pilch-Bisson) — Tue, 28 Oct 2008 17:58:19 GMT

Yes, realize works there, and I do like the point about intent over mechanism.

I chose the no arg "Apply" in deference to Jon Pryor (http://www.jprl.com/Blog/), who I've discussed the "Apply" method with a bunch of times.

re: Removing elements from a Dictionary

Jay Bazuzi — Tue, 28 Oct 2008 17:30:17 GMT

You talked about naming the no-action 'Apply()' overload 'Realize()'. I think a different name is a good idea, because it really does something quite different than the other 'Apply()'

What if you write it like this:

public static IEnumerable<T> Apply<T>(this IEnumerable<T> source)

{

return source.ToList();

}

And then replace your 'ToArray()' call with

'Realize()'?

I think this makes your code a little closer to intent, a little further from mechanism.