Merging IDictionary - is there a more efficient method than this?

Question

I have a helper that merges two or more IDictionary<TKey, TValue> objects into one IDictionary<TKey, string> by concatenating the TValue's ToString() methods like so:

public class DictionaryHelper<TKey, TValue>
{
    public static IDictionary<TKey, string> MergeDictionaries<TKey, TValue>(params IDictionary<TKey, TValue>[] dictionaries) where TValue : class
    {
        var returnValue = new Dictionary<TKey, string>();
        foreach (var dictionary in dictionaries)
        {
            foreach (var kvp in dictionary)
            {
                if (returnValue.ContainsKey(kvp.Key))
                {
                    returnValue[kvp.Key] += kvp.Value.ToString();
                }
                else
                {
                    returnValue[kvp.Key] = kvp.Value.ToString();
                }
            }
        }
        return returnValue;
    }

}

While this is straightforward and pretty easy to read, it seems like there should be a more efficient way to do this. Is there?

Keep in mind [`kvp.Value` can be `null`.](http://msdn.microsoft.com/en-us/library/k7z0zy8k.aspx) Efficient speed or efficient in lines of code? — user7116, May 01 '12 at 13:59
Hmm... well, my intended usage shouldn't allow that, but I agree I need to handle that. Thanks for pointing that out. Efficient in performance... I don't mind putting a bit more effort in up front. — Jeremy Holovacs, May 01 '12 at 14:01
This may be helpful: http://stackoverflow.com/questions/712927/how-to-add-2-dictionary-contents-without-looping-in-c-sharp — Steven Doggart, May 01 '12 at 14:02

score 7 · Accepted Answer · answered May 01 '12 at 14:07

7

I don't know if this is more efficient, but at least it's much shorter:

var result = dictionaries.SelectMany(d => d)
                         .ToLookup(kvp => kvp.Key, kvp => kvp.Value)
                         .ToDictionary(g => g.Key, g => string.Concat(g));

answered May 01 '12 at 14:07

dtb

213,145
36
401
431

@JeremyHolovacs: if we use yours as a baseline, they're all about even. (*previous version of this comment included deferred dictionary creation in the timing*) – user7116 May 01 '12 at 15:12
Interesting. So the compiler treats them the same, more or less. Then it becomes all about readability. – Jeremy Holovacs May 01 '12 at 15:18
1

@Jeremy: It meant slower, but I'd included deferred object creation in the timing erroneously. As you increase the size of the dictionary and the number of matches, dtb's takes a *slight* advantage over your code (1.05 speedup). – user7116 May 01 '12 at 15:19

score 1 · Answer 2 · edited May 23 '17 at 11:57

1

You could remove a visible foreach with SelectMany:

foreach (var kvp in dictionaries.SelectMany(dd => dd))
{
    if (returnValue.ContainsKey(kvp.Key))
    {
        returnValue[kvp.Key] += kvp.Value.ToString();
    }
    else
    {
        returnValue[kvp.Key] = kvp.Value.ToString();
    }
}

And you could extend this further, although dtb's is more elegant and efficient:

var merged = dictionaries.SelectMany(dd => dd)
                         .GroupBy(kvp => kvp.Key, kvp => kvp.Value)
                         .ToDictionary(
                             gg => gg.Key,
                             gg => String.Concat(gg));

However, this is not likely to be more efficient or readable than your current approach.

edited May 23 '17 at 11:57

Community

1
1

answered May 01 '12 at 14:06

user7116

63,008
17
141
172

You could replace `.GroupBy(kvp => kvp.Key).ToDictionary(..)` with `.GroupBy(kvp => kvp.Key, kvp => kvp.Value).ToDictionary(g => g.Key, g => string.Concat(g))`. So it's exactly the same as my answer except for GroupBy instead of ToLookup. I wonder which is more efficient... – dtb May 01 '12 at 14:14
Thanks, my gut says that L2O won't see a difference, and ILSpy shows using `GroupBy` in L2O simply defers a call to `Lookup.Create()` , which `ToLookup` uses immediately. Not sure there is much of a difference, except mine is making a `Lookup` on the fly and yours already has it. – user7116 May 01 '12 at 14:25

score 0 · Answer 3 · answered May 01 '12 at 14:01

0

You could use TryGetValue to conflate the location and retrieval of the source value.

answered May 01 '12 at 14:01

Steve Townsend

53,498
9
91
140

Merging IDictionary - is there a more efficient method than this?

3 Answers3