Mfcttrf

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.

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.

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.

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.

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.

IMHO form a purely theoretical point of view, TryGetValue and other ThisAndThat methods is a code smell by itself, it is a method/function that does more than one thing. If methods/functions were composable you would use one to get the value and one to decide what to replace the null with or to handle the exception. The API is flawed in 2019. But it is what it is.

The currently popular concept of Optional wrappers in every OO language is a more modern but just as flawed solution as it just moves the problem somewhere else.

There are much better design approaches

1. Validate the map/dictionary has the keys that are required in another method/function that is a precondition and do it once, and throw an exception that is basically an illegal argument mandatory key is missing with the name of the key so everyone knows exactly how to fix the problem.




2. Provide a DEFAULT map/dictionary that you copy the values from, then update that with whatever you are putting in there and you always are sure that the keys exist with a proper default and you can just let errors happen because they are indeed exceptional. Or use the DEFAULT map as an overlay map to fill in a sparse one with required keys with the proper default values.

The goal is to just avoid that TryGetValue mess, it is a code smell because it does more than one thing at time.