Skip navigation

This is a rant.  A brief rant.

How can any intelligent software developer think that throwing an exception when a key is not found with the [] operator is a good idea?

Come on.  The correct behavior should be that it simply returns null like every other collections API before (and hopefully after).

I should not have to write code like this:

object o = null; 

if (dictionary.ContainsKey(key) == true)  { o = dictionary[key]; }

Awful.

Advertisements

4 Comments

  1. This one actually has a purpose, because the value of a particular dictionary entry can be “null.” So, if myDictionary[“myKey”] returns null, we can’t tell whether or not somebody bothered to enter a value of null for “myKey”, or it just wasn’t entered at all yet.

    Unfortunately, as you allude to, the situation where people actually care which situation this is (or don’t know already based on prior code) is fairly rare, and could easily be identified with a “ContainsKey” check IF you actually care / don’t know how myDictionary[“myKey”] got to be null in the first place.

  2. Just use TryGetValue() method from Dictionary and you’ll be fine 😉

  3. I agree with Robert, just use TryGetValue. Also, with your original snippet, the ‘== true’ text is unnecessary as well since ContainsKey(key) returns a bool. TryGetValue does essentially the same thing, so:

    object o = null;
    if(dictionary.TryGetValue(key, out o))
    {
    // o is populated
    }
    else
    {
    // o is still null
    }

  4. The ‘== true’ is very necessary.

    Obviously the expression returns a bool or ‘== true’ would be a compile time error.

    I always, always expand the statement to include the bool comparison for two reasons:
    First, it’s a holdover from the C++ days. A common habit was to do something like:
    int* x = &y;
    if (x) { }
    In that snippet, it’s quite obvious that x is an int* because it’s declared next to where it’s used, but this is frequently not the case, especially when you’re dealing with members, and code written by other people. When, at first glance, it is not immediately obvious whether x is a bool or a pointer, you must guess. This is bad.
    Second, when reading code rapidly, it is much easier to read an expression that includes the “== true” then it is to read something like if (TryGetValue(SomeOtherFunction(), out x)), or god forbid, using the ! operator unless you are using it specifically to reverse a bool variable like in (x = !x).

    I am fully aware of the existence of TryGetValue. Let’s say that I had advocated this instead:

    object o = null;
    try
    {
    o = MyDictionary[“myKey”];
    }
    catch(KeyNotFoundException)
    {
    }

    Let’s suppose for a second that the bloated syntax doesn’t appall you. What else is wrong with this picture?

    Mostly that it fits the definition of throwing an exception for the purpose of flow control. One of the cardinal rules of good software is to never do that. Throwing an exception requires the .NET runtime to pause the active thread’s execution and build the call stack, which is a ridiculously expensive operation by comparison to returning false, or null.

    TryGetValue doesn’t throw an exception behind the scenes because the .NET develoeprs know this too.

    But I would counter the argument that it’s a good idea to throw a KeyNotFound exception rather than simply returning null as a way of discriminating between a missing key and a null value by suggesting that such a determination can be made just as easily by testing ContainsKey after the fact.

    I would also argue that being in the habit of populating a dictionary with nulls is probably not the best idea to begin with as it’s totally wasted space. The absence of the key should be the determinant, not the value being null. If you are writing code like:

    MyDictionary[“myKey”] = null;

    Why aren’t you instead writing:

    MyDictionary.Remove(“myKey”)?

    Both operations have the same asymptotic complexity.


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: