Should I always use Dictionary.TryGetValue even if I all want to do is add/replace values?

Question

I've been using TryGetValue to add/replace data in my dictionaries. Just to make the distinction between adding new vs. replacing old, I use both [] and .Add(). This leads to code like this, if I am not actually doing anything with the retrieved value:

private Dictionary<Foo, Bar> dictionary = new Dictionary<Foo, Bar>();

public void Update(Foo foo)
{
    Bar bar;
    if (dictionary.TryGetValue(foo, out bar)
    {
        dictionary [foo] = bar;
    }
    else
    {
        dictionary .Add(foo, bar);
    }
}

If I am not actually doing anything with the retrieved value, is there a reason why I shouldn't replace the above code with this?:

public void Update(Foo foo)
{
    dictionary[foo] = bar;
}

Thank you in advance.

_"Is there a reason why I shouldn't replace the above code with this?"_ **No** (apart from the fact that `bar` is neither declared nor ininitialzed) — Tim Schmelter, Apr 24 '13 at 15:15
Just don't do the reverse with the expectation of getting null for missing keys: `var item = dictionary[foo]` as this will throw an exception if the key doesn't exist (unlike `Hashtable`). — Adam Houldsworth, Apr 24 '13 at 15:16

score 5 · Accepted Answer · answered Apr 24 '13 at 15:13

5

You should use the simpler code.
It will be faster in all cases. (one hash lookup vs. two)

More importantly, it's simpler.

answered Apr 24 '13 at 15:13

SLaks

868,454
176
1,908
1,964

1

But the semantics is not the same (possible `KeyNotFoundException`) – Lukasz Madon Apr 24 '13 at 15:18
2

@lukas The semantics are the same as the alternative code sample, which wouldn't throw a `KeyNotFoundException` either, so in this case the simpler code achieves exactly the same. – Adam Houldsworth Apr 24 '13 at 15:19
1

@lukas: Neither snippet can possibly throw a `KeyNotFoundException`. – Daniel Hilgarth Apr 24 '13 at 15:20
@lukas: Wrong. The dictionary setter will create missing keys. – SLaks Apr 24 '13 at 15:26

Daniel Hilgarth · Answer 2 · 2013-04-24T16:06:04.353

4

No, there is no reason to not use the shorter code.
In fact, you really should use it, as it is much simpler and we all know:
Simpler code means less errors.

edited Apr 24 '13 at 16:06

answered Apr 24 '13 at 15:13

Daniel Hilgarth

171,043
40
335
443

You're taking for granted that the value exists in the collection, what if does not? If the try.. is removed from the method, the logic is all changed, because the method is not just updating, it is also checking if the key exists. – MeTitus Apr 24 '13 at 15:27
@Marco The setter on the indexer adds the key and the value if it is missing, or updates the value if the key exists. – Adam Houldsworth Apr 24 '13 at 15:28
@Marco: Please, go and read the [documentation](http://msdn.microsoft.com/en-us/library/9tee9ht2.aspx). The important part is this: "If the specified key is not found, [...] a set operation creates a new element with the specified key." – Daniel Hilgarth Apr 24 '13 at 15:28
@DanielHilgarth you're right. I'm quite surprise it works this way, I was pretty sure it would raise an exception if the item did not exist, hence and answer. – MeTitus Apr 24 '13 at 15:36
@Marco: Not a problem. Although I have to admit that the tone of your first comment on your answer really annoyed me. – Daniel Hilgarth Apr 24 '13 at 15:39
@DanielHilgarth I really thought it would raise an exception, that's I was not getting your comment. sorry about Daniel. If you edit your answer I will be able to remove my down vote. – MeTitus Apr 24 '13 at 15:41
@Marco: I improved the answer a bit. – Daniel Hilgarth Apr 24 '13 at 16:06

Should I always use Dictionary.TryGetValue even if I all want to do is add/replace values?

2 Answers2