Which way is thread-safe and has better performance to use, ContainsKey or string indexer?

Question

I have a collection as below

private static readonly Dictionary<string,object> _AppCache = new Dictionary<string,object>;

Then I was wondering which one is better to use to check if a key exists (none of my keys has null value)

1. _AppCache.ContainsKey("x")
2. _AppCache["x"] != null

This code might be access through various number of threads

The whole code is:

public void SetGlobalObject(string key, object value)
{
    globalCacheLock.EnterWriteLock();
    try
    {
        if (!_AppCache.ContainsKey(key))
        {
            _AppCache.Add(key, value);
        }
    }
    finally
    {
        globalCacheLock.ExitWriteLock();
    }
}

Update

I changed my code to use dictionary to keep focus of the question on Conatinskey or Indexer

Answer 1

I don't disagree with other's advice to use Dictionary. However, to answer your question, I think you should use ContainsKey to check if a key exists for several reasons

• That is specifically what ContainsKey was written to do
• For _AppCache["x"] != null to work your app must operate under an unenforced assumption (that no values will be null). That assumption may hold true now, but future maintainers may not know or understand this critical assumption, resulting in unintuitive bugs
• Slightly less processing for ContainsKey, although this is not really important

Neither of the two choices are threadsafe, so that is not a deciding factor. For that, you either need to use locking, or use ConcurrentDictionary.

If you move to a Dictionary (per your question update), the answer is even more in favor of ContainsKey. If you used the index option, you would have to catch an exception to detect if the key is not in the Dictionary. ContainsKey would be much more straightforward in your code.

When the key is in the Dictionary, ContainsKey is slightly more efficient. Both options first call an internal method FindEntry. In the case of ContainsKey, it just returns the result of that. For the index option, it must also retrieve the value. In the case of the key not being in the Dictionary, the index option would be a fair amount less efficient, because it will be throwing an exception.

Answer 2

You are obviously checking for the existence of that key. In that case, _AppCache["x"] != null will give you a KeyNotFoundException if the key does not exist, which is probably not as desirable. If you really want to check if the key exists, without generating an exception by just checking, you have to use _AppCache.ContainsKey("x"). For checking if the key exists in the dictionary or hashtable, I would stick with ContainsKey. Any difference in performance, if != null is faster, would be offset by the additional code to deal with the exception if the key really does not exist.

In reality, _AppCache["x"] != null is not checking if the key exists, it is checking, given that key "x" exists, whether the associated value is null.

Neither way (although accomplishing different tasks) gives you any advantage on thread safety.

All of this holds true if you use ConcurrentDictionary - no difference in thread safety, the two ways accomplish different things, any possible gain in checking with !=null is offset by additional code to handle exception. So, use ContainsKey.

Answer 3

If you're concerned about thread-safety, you should have a look at the ConcurrentDictionary class.

If you do not want to use ConcurrentDictionary, than you'll have to make sure that you synchronize access to your regular Dictionary<K,V> instance. That means, making sure that no 2 threads can have multiple access to your dictionary, by locking on each write and read operation.

For instance, if you want to add something to a regular Dictionary in a thread-safe way, you'll have to do it like this:

private readonly object _sync = new object();

// ...


lock( _sync )
{
    if( _dictionary.ContainsKey(someKey) == false )
    {
         _dictionary.Add(someKey, somevalue);
    }
}

You should'nt be using using Hashtable anymore since the introduction of the generic Dictionary<K,V> class and therefore type-safe alternative has been introduced in .NET 2.0

One caveat though when using a Dictionary<K,V>: when you want to retrieve the value associated with a given key, the Dictionary will throw an exception when there is no entry for that specified key, whereas a Hashtable will return null in that case.

Answer 4

You should use a ConcurrentDictionary rather than a Dictionary, which is thread-safe itself. Therefore you do not need the lock, which (generally *) improves performance, since the locking mechanisms are rather expensive.

Now, only to check whether an entry exists I recommend ContainsKey, irrespective of which (Concurrent)Dictionary you use:

_AppCache.ContainsKey(key)

But what you do in two steps can be done in one step using the Concurrent Dictionary by using GetOrAdd:

_AppCache.GetOrAdd(key, value);

You need a lock for neither action:

public void SetGlobalObject(string key, object value)
{
    _AppCache.GetOrAdd(key, value);
}

Not only does this (probably *) perform better, but I think it expresses your intentions much clearer and less cluttered.

(*) Using "probably" and "generally" here to emphasise that these data structures do have loads of baked-in optimisations for performance, however performance in your specific case must always be measured.