Adding to a dictionary and skipping duplicate keys

Question

What is the cleanest way to use dictionaries and protect against an unhandled ArgumentException if the key already exists in the dictionary?

I'd like to say that I can always guarantee uniqueness upstream, but I'm dealing with legacy code and mildly corrupt data from an old bug in the system.

I find myself using this pattern frequently but wonder if there is a better way.

public void AddToDictionaryContainsKey(Dictionary<int,string> myDictionary, int myKey, string myValue )
{
    if (!myDictionary.ContainsKey(myKey))
    {
        myDictionary.Add(myKey, myValue);
    }
}

Maybe a try/catch block - but this seems less readable. The advantage is that if I ever do decide to log the problem I have a place to do it.

public void AddToDictionaryTryCatch(Dictionary<int, string> myDictionary, int myKey, string myValue)
{
    try
    {
        myDictionary.Add(myKey, myValue);
    }
    catch (ArgumentException e)
    {
        // keep on truckin'
    }
}

The dictionaries are relatively small (typically < 1000 entries) so I prefer readability over performance.

Answer 1

Generating and handling exceptions is considered to be an expensive operation, so if(! x.ContainsKey()) is better. Yeah, the code example I see in MSDN uses try/catch but that's to illustrate the exception not advocate that as "best practice." Documentation I've read is pretty adamant about not throwing exceptions needlessly.

And you don't need try/catch to trap the duplicate key; just an else to the above if.

Answer 2

How about creating an extension method..

static class Extensions
{
    public static void AddSafe(this Dictionary<int, string> dictionary, int key, string value)
    {
        if (!dictionary.ContainsKey(key))
            dictionary.Add(key, value);
    }
}

and calling it like this:

var students = new Dictionary<int, string>();
students.AddSafe(1, "Apple");
students.AddSafe(1, "Orange");

Answer 3

Something is missing in your question: does a duplicate entry is an error or a special case? This is the only question you should ask yourself.

If duplicates are not expected, use an exception (and don't catch it at this level). Performance is not an issue when an error occurs.

Otherwise, don't use exceptions. This would deceive other developers (they will think that a duplicate IS an error). It would also bother them each time they'll ask Visual Studio to stop on each exception.

Answer 4

I would go with ContainsKey, but take it one step further and make it a generic extension method

public static void AddIfKeyUnique<TKey, TValue>(this Dictionary<TKey, TValue> dictionary, TKey key, TValue value)
{
    if (!dictionary.ContainsKey(key))
    {
            dictionary.Add(key, value);
    }
}

Answer 5

The ContainsKey way is cleaner I think so I prefer it in any way.

About performance, it's depend if your case, take in mind that check for contains key and then add key is do almost the same twice. So if all add operations result as success, the try\catch will be faster also if just some false occurred, but in the most case, the ContainsKey will be faster.

Answer 6

Where you already have had for years excellent answers on what the idiomatic best practice is for the given use case, as a developer you will often find yourself needing to tweak that code somewhat in different situations (e.g. different key or value types in a different dictionary, or maybe even a different dictionary class altogether) and rather than re-writing every instance where the pattern is used, you'll want to DRY (don't repeat yourself) your code out to have a minimal, well-tested area where this particular functionality and behavior resides. To that end, here's a very small extension method class that a) uses IDictionary rather than Dictionary to develop to interfaces rather than implementations, b) adds generics to the mix rather than the int and string concretions, c) returns a bool in the case you need to know if a the add made it in or not, d) utilizes overloads such that either the key and value may be used directly, or any mixture of Funcs that return those parameters can be used.

public static class IDictionaryExtensions
{
    public static bool TryAdd<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, TValue value)
    {
        if (dictionary?.ContainsKey(key) == false)
        {
            dictionary.Add(key, value);
            return true;
        }

        return false;
    }
    
    public static bool TryAdd<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, Func<TKey> keyFunc, TValue value) =>
        dictionary.TryAdd(keyFunc(), value);
    
    public static bool TryAdd<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, Func<TValue> valueFunc) =>
        dictionary.TryAdd(key, valueFunc());
    
    public static bool TryAdd<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, Func<TKey> keyFunc, Func<TValue> valueFunc) =>
        dictionary.TryAdd(keyFunc(), valueFunc());
}