23

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. ...


22

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 ...


12

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'...


10

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: ...


9

You are using your languages method overloading feature to offer the caller alternate ways of resolving the methods dependency on positional information. You are then delegating to another method to resolve the remaining work of updating the camera. The code smell here would be if you keep just extending the chain of methods calling methods. The Location ...


8

This is not feature envy but questionnable design: Feature envy is about methods of one class using data of another class "excessively". It is a symptom of a potential design issue, e.g. that methods should belong to the other class, or that methods don't use abstractions offered by the other class as they should. Your design intends to use a pure data ...


8

There is nothing particularly wrong with your solution. But my personal preference would be that those methods are not that useful. And just complicate the interface of whatever object they are part off. The void moveCameraTo(double latitude, double longitude) doesn't really simplify the code, as I see no problem simply calling moveCameraTo(new LatLng(...


6

Generally, the point of OOP is that you don't need access to the attribute. Think about what you want to do with the attribute - whatever that is, should potentially be a method of the class, so you should call that method to do it, instead of getting the attribute value and doing it yourself. There are exceptions, of course, but if all your 'classes' are ...


5

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) ...


4

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.


4

Short answer: yes, it is a good approach in most cases. It has a well-known name, it is called creating abstractions, and it is one of the fundamental basics of program design. In this case, a class like DataStorageService provides functions with meaningful, specific names for certain actions, hiding away the implementation in terms of functions with ...


4

A good argument why by Martin Fowler: "Tell-Don't-Ask is a principle that helps people remember that object-orientation is about bundling data with the functions that operate on that data. It reminds us that rather than asking an object for data and acting on that data, we should instead tell an object what to do. This encourages to move behavior into an ...


3

You need to think about how the system interacts with its environment. That is you need to think about your scope boundary API. How is the developer supposed to use this? First of all, is the code using Database responsible of tracking where objects are? No, right, it isn't. Thus Database must have an API that allows to put an object in a Location ...


2

What you need to understand is that there are two kinds of objects: Objects that have behavior. These should refrain from giving public access to most/any of their data members. I expect only very few accessor methods defined for these. An example would be a compiled regex: The object is created to provide a certain behavior (to match a string against a ...


2

Hopefully the integer values correspond to some meaningful names, like an action of "create" and a status of "complete". Your code would be clearer if it used these values (either as Strings or constants) instead of magic integers. The "divide and conquer" and do something like: switch(action) { case CREATE: handleCreate(status); break; case DELETE: ...


2

Depending on the language you use, you can switch on composite types. If you can't do that, you might be able to map your composite onto a primitive, e.g. 10 * action + status. You'd have something like switch (ActionStatus(action, status)) { case ActionStatus(1, 1): case ActionStatus(1, 3): case ActionStatus(3, 1): { doThing1(); break; } ...


2

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 ...


2

Let's say I have a class that's a bit more than a pure Data object... That's good, because there is no such thing as a "data object". A data object is a record or struct. At least in an object-oriented environment those are a good indication of a bad design in themselves. It is bad, because inevitably provokes either Feature Envy, as you point our or just ...


2

"several parameters" means you will likely forget which position is which if you set these with a constructor. There are ways around this problem but they depend on your language. If you use a language with named arguments like C# you can fix that problem: new Lexer( whitespaceSplit:true, otherParameter:"other" ); If you use a language without ...


1

Before giving my opinion and suggestion, it is better to apprehend the definition from the previous cases: Methods that make extensive use of another class may belong in another class. Consider moving this method to the class it is so envious of. https://blog.codinghorror.com/code-smells/ with that quote in mind and assuming a simple logic of your ...


1

What about letting those parameters which change the parsers behavior only to be passed with the construction of the instance? class Lexer { public: constructor Lexer(whitespaceSplit : boolean; otherParameter : ...) : whitespaceSplit_(whitespaceSplit ), otherParameter_(otherParameter) { } property whitespaceSplit : boolean read ...


1

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 ...


1

If you mind using kind of methods, it is just method-overloading feature it is good. But there is not eliminate the responsibility of knowing what is a LatLng. Because you are initializing LatLng latLng = new LatLng(latitude, longitude). This is totatly dependency to LatLng. (To understand why Initializing is a dependency problem, you can check Dependency ...


1

I'm not sure what the proper name for this is, if it has one, but it is good practice. You expose multiple overloads, allowing the calling class to determine which parameter set it wants to use. As you say, another class may not know what a LatLng object is, but might know its Location. Having one method call the other is key too, as you don't want ...


1

I need a limited amount of objects = instances from a class, and I don't want to expose the option to create more. I also want easy access to them from everywhere That is an unnecessary constraint that seems motivated by apriori notions of how you can design and implement a game of chess -- be it a two player chess game or a three player chess game. What ...


Only top voted, non community-wiki answers of a minimum length are eligible