Should I check whether particular key is present in Dictionary before accessing it?

Question

Should I check whether particular key is present in Dictionary if I am sure it will be added in dictionary by the time I reach the code to access it?

There are two ways I can access the value in dictionary

1. checking ContainsKey method. If it returns true then I access using indexer [key] of dictionary object.

or

1. TryGetValue which will return true or false as well as return value through out parameter.

(2nd will perform better than 1st if I want to get value. Benchmark.)

However if I am sure that the function which is accessing global dictionary will surely have the key then should I still check using TryGetValue or without checking I should use indexer[].

Or I should never assume that and always check?

Answer 1

If the dictionary is global (static/shared), you should be synchronizing access to it (this is important; otherwise you can corrupt it).

Even if your thread is only reading data, it needs to respect the locks of other threads that might be editing it.

However; if you are sure that the item is there, the indexer should be fine:

Foo foo;
lock(syncLock) {
    foo = data[key];
}
// use foo...

Otherwise, a useful pattern is to check and add in the same lock:

Foo foo;
lock(syncLock) {
    if(!data.TryGetValue(key, out foo)) {
        foo = new Foo(key);
        data.Add(key, foo);
    }
}
// use foo...

Here we only add the item if it wasn't there... but inside the same lock.

Answer 2

Always check. Never say never. I assume your application is not that performance critical that you will have to save the checking time.

TIP: If you decide not to check, at least use Debug.Assert( dict.ContainsKey( key ) ); This will only be compiled when in Debug mode, your release build will not contain it. That way you could at least have the check when debugging.

Still: if possible, just check it :-)

EDIT: There have been some misconceptions here. By "always check" I did not only mean using an if somewhere. Handling an exception properly was also included in this. So, to be more precise: never take anything for granted, expect the unexpected. Check by ContainsKey or handle the potential exception, but do SOMETHING in case the element is not contained.

Answer 3

Use the indexer if the key is meant to be present - if it's not present, it will throw an appropriate exception, which is the right behaviour if the absence of the key indicates a bug.

If it's valid for the key not to be present, use TryGetValue instead and react accordingly.

(Also apply Marc's advice about accessing a shared dictionary safely.)

Answer 4

TryGetValue is the same code as indexing it by key, except the former returns a default value (for the out parameter) where the latter throws an exception. Use TryGetValue and you'll get consistent checks with absolutely no performance loss.

Edit: As Jon said, if you know it will always have the key, then you can index it and let it throw the appropriate exception. However, if you can provide better context information by throwing it yourself with a detailed message, that would be preferable.

Answer 5

There's 2 trains of thought on this from a performance point of view.

1) Avoid exceptions where possible, as exceptions are expensive - i.e. check before you try to retrieve a specific key from the dictionary, whether it exists or not. Better approach in my opinion if there's a fair chance it may not exist. This would prevent fairly common exceptions.

2) If you're confident the item will exist in there 99% of the time, then don't check for it's existence before accessing it. The 1% of times when it doesn't exist, an exception will be thrown but you've saved time for the other 99% of the time by not checking.

What I'm saying is, optimise for the majority if there is a clear one. If there is any real degree in uncertainty about an item existing, then check before retrieving.

Answer 6

If you know that the dictionary normally contains the key, you don't have to check for it before accessing it.

If something would be wrong and the dictionary doesn't contain the items that it should, you can let the dictionary throw the exception. The only reason for checking for the key first would be if you want to take care of this problem situation yourself without getting the exception. Letting the dictionary throw the exception and catch that is however a perfectly valid way of handling the situation.

Answer 7

I think Marc and Jon have it (as usual) pretty sown up. Since you also mention performance in your question it might be worth considering how you lock the dictionary.

The straightforward lock serialises all read access which may not be desirable if read is massively frequent and writes are relatively few. In that case using a ReaderWriterLockSlim might be better. The downside is the code is a little more complex and writes are slightly slower.

Answer 8

Personally I'd check the key is there, regardless of whether or not you are SURE it is, some may say this check is superfluous and that dictionary will throw an exception which you can catch, but imho you should not rely on that exception, you should check yourself and then either throw your own exception which means something or a result object with a success flag and reason inside... the failure mechanism is really implementation dependant.