33

I find myself often looking up questions online, and many solutions include dictionaries. However, whenever I try to implement them, I get this horrible reek in my code. For example every time I want to use a value:

int x;
if (dict.TryGetValue("key", out x)) {
    DoSomethingWith(x);
}

That's 4 lines of code to essentially do the following: DoSomethingWith(dict["key"])

I've heard that using the out keyword is an anti pattern because it makes functions mutate their parameters.

Also, I find myself often needing a "reversed" dictionary, where I flip the keys and values.

Similarly, I often would like to iterate through the items in a dictionary and find myself converting keys or values to lists etc to do this better.

I feel like there's almost always a better, more elegant way of using dictionaries, But I'm at a loss.

18
  • 9
    Other ways may exist but I generally use a ContainsKey first before trying to get a value.
    – Robbie Dee
    Aug 27, 2019 at 15:53
  • 7
    @RobbieDee: you have to be careful when you do that though, as doing so creates a race condition. It's possible the key could be removed between calling ContainsKey and getting the value. Aug 27, 2019 at 18:24
  • 14
    @whatsisname: Under those conditions, a ConcurrentDictionary would be more suitable. Collections in the System.Collections.Generic namespace are not thread-safe. Aug 27, 2019 at 20:49
  • 4
    A good 50% of the time I see someone using a Dictionary, what they really wanted was a new class. For those 50% (only), it's a design smell. Aug 28, 2019 at 0:59
  • 4
    OT: You can use foo.TryGetValue("key", out int x) in never C# versions to save the initial variable declaration. Aug 28, 2019 at 9:14

9 Answers 9

29

Dictionaries (C# or otherwise) are simply a container where you look up a value based on a key. In many languages it's more correctly identified as a Map with the most common implementation being a HashMap.

The problem to consider is what happens when a key does not exist. Some languages behave by returning null or nil or some other equivalent value. Silently defaulting to a value instead of informing you that a value does not exist.

For better or for worse, the C# library designers came up with an idiom to deal with the behavior. They reasoned that the default behavior for looking up a value that does not exist is to throw an exception. If you want to avoid exceptions, then you can use the Try variant. It's the same approach they use for parsing strings into integers or date/time objects. Essentially, the impact is like this:

T count = int.Parse("12T45"); // throws exception

if (int.TryParse("12T45", out count))
{
    // Does not throw exception
}

And that carried forward to the dictionary, whose indexer delegates to Get(index):

var myvalue = dict["12345"]; // throws exception
myvalue = dict.Get("12345"); // throws exception

if (dict.TryGet("12345", out myvalue))
{
    // Does not throw exception
}

This is simply the way the language is designed.


Should out variables be discouraged?

C# isn't the first language to have them, and they have their purpose in specific situations. If you are trying to build a highly concurrent system, then you cannot use out variables at the concurrency boundaries.

In many ways, if there is an idiom that is espoused by the language and core library providers, I try to adopt those idioms in my APIs. That makes the API feel more consistent and at home in that language. So a method written in Ruby isn't going to look like a method written in C#, C, or Python. They each have a preferred way of building code, and working with that helps the users of your API learn it more quickly.


Are Maps in General an Anti-pattern?

They have their purpose, but many times they may be the wrong solution for the purpose you have. Particularly if you have a bi-directional mapping you need. There are many containers and ways of organizing data. There are many approaches that you can use, and sometimes you need to think a bit before you pick that container.

If you have a very short list of bi-directional mapping values, then you might only need a list of tuples. Or a list of structs, where you can just as easily find the first match on either side of the mapping.

Think of the problem domain, and pick the most appropriate tool for the job. If there isn't one, then create it.

8
  • 4
    "then you might only need a list of tuples. Or a list of structs, where you can just as easily find the first match on either side of the mapping." -- If there are optimizations for small sets, they should be made in the library, not rubber-stamped into user code.
    – Blrfl
    Aug 27, 2019 at 20:50
  • 3
    It is, but if the behavior is still the same, that determination should be in the library. I've run across containers implementations in other languages that will switch algorithms if told a priori that the number of entries will be small. I agree with your answer, but once this has been figured out, it should be in a library, maybe as a SmallBidirectionalMap.
    – Blrfl
    Aug 27, 2019 at 22:52
  • 7
    "For better or for worse, the C# library designers came up with an idiom to deal with the behavior." – I think you hit the nail on the head: they "came up" with an idiom, instead of using an existing widely-used one, which is an optional return type. Aug 28, 2019 at 8:46
  • 3
    @JörgWMittag If you're talking about something like Option<T>, then I think that would make the method much harder to use in C# 2.0, because it didn't have pattern matching.
    – svick
    Aug 28, 2019 at 16:21
  • 1
    I'm of the opinion out variables should be discouraged, that said its only a readability preference not a dangerous anti-pattern. TryGet has a significant peformance benefit in that it does the key/value search only once, doing a contains call followed by a seperate get is going to be around half the speed. Oct 20, 2021 at 14:27
34

Some good answers here on the general principles of hashtables/dictionaries. But I thought I'd touch on your code example,

int x;
if (dict.TryGetValue("key", out x)) 
{
    DoSomethingWith(x);
}

As of C# 7 (which I think is around two years old), that can be simplified to:

if (dict.TryGetValue("key", out var x))
{
    DoSomethingWith(x);
}

And of course it could be reduced to one line:

if (dict.TryGetValue("key", out var x)) DoSomethingWith(x);

If you have a default value for when the key doesn't exist, it can become:

DoSomethingWith(dict.TryGetValue("key", out var x) ? x : defaultValue);

So you can achieve compact forms by using reasonably recent language additions.

7
  • 2
    Good call on the v7 syntax, nice little option to have to cut out the extra definition line while allowing the use of var +1
    – BrianH
    Aug 28, 2019 at 0:07
  • 2
    Note also that if "key" is not present, x will be initialized to default(TValue) Aug 28, 2019 at 4:16
  • Might be nice as a generic extension, too, invoked like "key".DoSomethingWithThis() Aug 28, 2019 at 20:11
  • 2
    I feel like this answer deserves a disclaimer that writing compact code for the sake of it being compact is bad. It can make your code much harder to read. Additionally, if I recall correctly, TryGetValue is not atomic/thread safe so you could just as easily perform one check for existence and another to grab and operate on the value
    – Marie
    Sep 19, 2019 at 19:43
  • 1
    @Marie, indeed the whole of the Dictionary<TKey, TValue> class is not thread-safe. Use the ConcurrentDictionary<TKey, TValue> type if thread safety is needed.
    – David Arno
    Sep 20, 2019 at 7:28
15

This is neither a code smell nor an anti-pattern, as using TryGet-style functions with an out parameter is idiomatic C#. However, there are 3 options provided in C# to work with a Dictionary, so should you be sure you are using the correct one for your situation. I think I know where the rumor of there being a problem using an out parameter comes from, so I'll handle that at the end.

What features to use when working with a C# Dictionary:

  1. If you are sure the key will be in the Dictionary, use the Item[TKey] property
  2. If the key should generally be in the Dictionary, but it is bad/rare/problematic that it is not there, you should use Try...Catch so that an error will be raised and then you can try to handle the error gracefully
  3. If you are not sure the key will be in the Dictionary, use TryGet with the out parameter

To justify this, one need only refer to the documentation for Dictionary TryGetValue, under "Remarks":

This method combines the functionality of the ContainsKey method and the Item[TKey] property.

...

Use the TryGetValue method if your code frequently attempts to access keys that are not in the dictionary. Using this method is more efficient than catching the KeyNotFoundException thrown by the Item[TKey] property.

This method approaches an O(1) operation.

The whole reason TryGetValue exists is to act as a more convenient way to use ContainsKey and Item[TKey], while avoiding having to search the dictionary twice - so pretending it doesn't exist and doing the two things it does manually is a rather awkward choice.

In practice, I rarely have ever used a raw Dictionary, due to this simple maxim: choose the most generic class/container that gives you the functionality you need. Dictionary was not designed to look up by value rather than by key (for example), so if that is something you want it may make more sense to use an alternate structure. I think I may have used Dictionary one time in the last year-long development project I did, simply because it was rarely the right tool for the job I was trying to do. Dictionary is certainly not the Swiss Army knife of the C# toolbox.

What's wrong with out parameters?

CA1021: Avoid out parameters

Although return values are commonplace and heavily used, the correct application of out and ref parameters requires intermediate design and coding skills. Library architects who design for a general audience should not expect users to master working with out or ref parameters.

I'm guessing that's where you heard that out parameters were something like an anti-pattern. As with all rules, you should read closer to understand the 'why', and in this case there is even an explicit mention of how the Try pattern does not violate the rule:

Methods that implement the Try pattern, such as System.Int32.TryParse, do not raise this violation.

1
12

There are at least two methods missing from C# dictionaries that in my opinion clean up code considerably in a lot of situations in other languages. The first is returning an Option, which lets you write code like the following in Scala:

dict.get("key").map(doSomethingWith)

The second is returning a user-specified default value if the key isn't found:

doSomethingWith(dict.getOrElse("key", "key not found"))

There is something to be said for using the idioms a language provides when appropriate, like the Try pattern, but that doesn't mean you have to only use what the language provides. We're programmers. It's okay to create new abstractions to make our specific situation easier to understand, especially if it eliminates a lot of repetition. If you frequently need something, like reverse lookups or iterating through values, make it happen. Create the interface you wish you had.

2
  • I like the 2nd option. Maybe I'll have to write an extension method or two :)
    – Adam B
    Aug 27, 2019 at 17:26
  • 4
    on the plus side c# extension methods means you can implement these yourself
    – jk.
    Aug 28, 2019 at 9:46
7

That's 4 lines of code to essentially do the following: DoSomethingWith(dict["key"])

I agree that this is inelegant. A mechanism that I like to use in this case, where the value is a struct type, is:

public static V? TryGetValue<K, V>(
      this Dictionary<K, V> dict, K key) where V : struct => 
  dict.TryGetValue(key, out V v) ? new V?(v) : new V?();

Now we have a new version of TryGetValue that returns int?. We can then do a similar trick for extending T?:

public static void DoIt<T>(
      this T? item, Action<T> action) where T : struct
{
  if (item != null) action(item.GetValueOrDefault());
}

And now put it together:

dict.TryGetValue("key").DoIt(DoSomethingWith);

and we're down to a single, clear statement.

I've heard that using the out keyword is an anti pattern because it makes functions mutate their parameters.

I would be slightly less strongly worded, and say that avoiding mutations when possible is a good idea.

I find myself often needing a "reversed" dictionary, where I flip the keys and values.

Then implement or obtain a bidirectional dictionary. They are straightforward to write, or there are plenty of implementations available on the internet. There are a bunch of implementations here, for example:

https://stackoverflow.com/questions/268321/bidirectional-1-to-1-dictionary-in-c-sharp

Similarly, I often would like to iterate through the items in a dictionary and find myself converting keys or values to lists etc to do this better.

Sure, we all do.

I feel like there's almost always a better, more elegant way of using dictionaries, But I'm at a loss.

Ask yourself "suppose I had a class other than Dictionary that implemented the exact operation that I wish to do; what would that class look like?" Then, once you've answered that question, implement that class. You're a computer programmer. Solve your problems by writing computer programs!

6
  • Thanks. Do you think you could go into a bit more explanation of what the action method is actually doing? I’m new to actions.
    – Adam B
    Aug 29, 2019 at 4:07
  • 1
    @AdamB an action is an object that represents the ability to call a void method. As you see, on the caller side we pass the name of a void method whose parameter list matches the generic type arguments of the action. On the caller side the action is invoked like any other method. Aug 29, 2019 at 14:19
  • 1
    @AdamB: You can think of Action<T> as being very similar to interface IAction<T> { void Invoke(T t); }, but with very lenient rules about what "implements" that interface, and about how Invoke may be invoked. If you want to know more, learn about "delegates" in C#, and then learn about lambda expressions. Aug 29, 2019 at 20:20
  • Ok I think I got it. So the action let’s you put a method in as the parameter for DoIt(). And calls that method.
    – Adam B
    Aug 29, 2019 at 20:35
  • @AdamB: Exactly right. This is an example of "functional programming with higher-order functions". Most functions take data as arguments; higher-order functions take functions as arguments and then do stuff with those functions. As you learn more C# you'll see that LINQ is entirely implemented with higher-order functions. Aug 29, 2019 at 20:51
3

The TryGetValue() construct is only necessary if you don't know whether "key" is present as a key within the dictionary or not, otherwise DoSomethingWith(dict["key"]) is perfectly valid.

A "less dirty" approach might be to use ContainsKey() as a check instead.

9
  • 6
    "A "less dirty" approach might be to use ContainsKey() as a check instead." I disagree. As suboptimal as TryGetValue is, at least it makes it hard to forget to handle the empty case. Ideally, I wish this would just return an Optional.
    – Alexander
    Aug 27, 2019 at 17:37
  • 1
    There's a potential problem with the ContainsKey approach for multithreaded applications, which is time of check to time of use (TOCTOU) vulnerability. What if some other thread deletes the key between the calls to ContainsKey and GetValue? Aug 28, 2019 at 10:41
  • 2
    @JAD Optionals compose. You can have an Optional<Optional<T>>
    – Alexander
    Aug 28, 2019 at 13:49
  • 2
    @DavidHammen To be clear, you would need TryGetValue on ConcurrentDictionary. The regular dictionary wouldn't be synchronized
    – Alexander
    Aug 28, 2019 at 13:50
  • 2
    @JAD No, (Java's optional type blows, don't get me started). I'm talking more abstractly
    – Alexander
    Aug 28, 2019 at 13:50
3

Other answers contain great points, so I won't restate them here, but instead I'll focus on this part, which seems to be largely ignored so far:

Similarly, I often would like to iterate through the items in a dictionary and find myself converting keys or values to lists etc to do this better.

Actually, it's pretty easy to iterate over a dictionary, since it does implement IEnumerable:

var dict = new Dictionary<int, string>();

foreach ( var item in dict ) {
    Console.WriteLine("{0} => {1}", item.Key, item.Value);
}

If you prefer Linq, that too works out just fine:

dict.Select(x=>x.Value).WhateverElseYouWant();

All in all, I don't find Dictionaries to be an antipattern - they're just a specific tool with specific uses.

Also, for even more specifics, check out SortedDictionary (uses R-B trees for a more predictable performance) and SortedList (which is also a confusingly named dictionary that sacrifices insertion speed for lookup speed, but shines if you're staring out with a fixed, pre-sorted set). I've had a case where replacing a Dictionary with a SortedDictionary resulted in an order of magnitude faster execution (but it could happen the other way round too).

2

If you feel using a dictionary is awkward it may not be the right choice for your problem. Dictionaries are great but like one commenter noticed, often they are used as a shortcut for something that should have been a class. Or the dictionary itself may be right as a core storage method but there should be a wrapper class around it to provide the desired service methods.

A lot has been said about Dict[key] versus TryGet. What I use a lot is iterating over the dictionary using KeyValuePair<TKey, TValue>. Apparently this is a less commonly known construct.

The main boon of a dictionary is that it is really fast compared to other collections as the number of items grows. If you often find yourself checking whether a key exists or not you may want to ask yourself if your use of a dictionary is appropriate. As a client you should typically know what you put in there and thus what would be safe to query.

7
  • really fast could do with some qualification
    – Caius Jard
    Oct 29, 2021 at 11:22
  • @CaiusJard Fast as in O(1). It practically doesn't matter how many items are in the collection, a requested item will be returned in a constant time that does not increase as the number of items grows. Oct 29, 2021 at 13:45
  • I was curious because it's used in a sentence talking about growth; there's the build/maintenance aspect too because a dictionary uses arrays internally, that it has to grow occasionally, and there's a reasonable amount more juggling that a dictionary does during growth that e.g. a list and e.e.g. definitely a linkedlist doesn't do. A minor consideration too is that, AIUI, colliding hashes essentially store in something more like a linked list, and it's not a straight out hash code collision but the modulo of the size of the dictionary's buckets that matters so you can have quite a lot of ..
    – Caius Jard
    Oct 29, 2021 at 15:38
  • .. linked list style behavior. (It would be unfortunate, but I believe it's entirely possible to have a dictionary where every element has the same modhash and hence it will behave like a list, i.e. O(n), because it will have to loop through every colliding item using Equals on the key to find the one you asked for)
    – Caius Jard
    Oct 29, 2021 at 15:39
  • @CaiusJard "possible to have a dictionary where every element has the same modhash " That is not a practical scenario. Hash collisions are rare, the number of items in a bucket will be small for any serious dictionary implementation and the increased access times negligible. You would need to create a dictionary with a humongous amount of items before this would become an issue. You will run out of memory sooner. Oct 29, 2021 at 17:02
1

This was bugging me too, so I came up with this extension:

public static class DictionaryExtension
{
    public static TValue GetValueOrDefault<TKey, TValue>(this Dictionary<TKey, TValue> dict, TKey key)
    {
        if (dict.TryGetValue(key, out var value))
        {
            return value;
        }

        return default;
    }
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.