views:

1043

answers:

5

I'm adding items to a StringDictionary and it's possible that a duplicate key will come up. This will of course throw an exception.

If the chance of duplicates is very low (ie it will rarely happen), am I better off using a Try Catch block and leaving it unhandled, or should I always do a .ContainsKey check before adding each entry?

I'm assuming that if the likelihood of duplicate keys was high, then allowing exceptions would be a poor decision as they are expensive.

Thoughts?

Edit

I used reflector on the generic Dictionary and found the following for ContainsKey and TryGetValue, as both were mentioned below.

public bool TryGetValue(TKey key, out TValue value)
{
    int index = this.FindEntry(key);
    if (index >= 0)
    {
        value = this.entries[index].value;
        return true;
    }
    value = default(TValue);
    return false;
}

And

public bool ContainsKey(TKey key)
{
    return (this.FindEntry(key) >= 0);
}

Am I missing something, or is TryGetValue doing more work than ContainsKey ?


I appreciate the responses, and for my current purpose I'm going to go with doing a ContainsKey call as the collection will be small, and the code more readable.

+4  A: 

How to approach this depends on what you want to do if a collision happens. If you want to keep the first inserted value, you should use ContainsKey to check before inserting. If, on the other hand, you want to use the last value for that key, you can do like so:

// c# sample:
myDictionary[key] = value;

As a side note: I would probably, if possible, use Dictionary<string, string> instead of StringDictionary. If nothing else that will give you access to some more Linq extension methods.

Fredrik Mörk
+4  A: 

I would do the Contains check.

My reasoning is exceptions should be saved for those things that just shouldn't happen. If they do then alarm bells should be rang and calvery brought in. Just seems odd to me to use exceptions for known issue case handling especially when you can test for it.

Kelsey
+1  A: 

Unless this is a very large dictionary or in a critical inner loop of code, you will probably not see a difference.

The .ContainsKey check will cost you a little performance every time, while the thrown exception will cost you a bit more performance rarely. If the chances of a duplicate are indeed low, go with allowing the Exception.

If you actually do want to be able to manage duplicate keys, you might look at MultiDictionary in PowerCollections

Eric J.
A: 

I try to avoid Exceptions everywhere I can - they're expensive to handle, and they can complicate the code. Since you know that a collision is possible, and it's trivial to do the .Contains check, I'd do that.

rwmnau
The problem is that `ContainsKey` check is also potentially expensive (exactly as expensive as retrieval will be - you end up with double key lookup). And it is not at all certain if, _for a case where no exception is thrown_ (which is claimed to be the typical case), exception handling overhead is going to be slower than the extra `Contains` check.
Pavel Minaev
+2  A: 

If at all possible, replace StringDictionary with Dictionary<string, string>, and use TryGetValue. This avoids both exception handling overhead, and double lookup.

Pavel Minaev
+1 - Ditto. Also think about thread safety if appropriate - you might check and find the key isn't there, but then another thread adds it before your add code executes and you get an exception.
TrueWill
TryGetValue() does a double lookup in its implementation... and it's not thread-safe
murki