2

At my company we have multiple C# programs that use Dictionaries in a config class to keep track of the configuration and state of the program.

For example,

bool isFoo = config.GetBool("Foo");

This seems clunky to me for a number of reasons. For one, there is no easy way of knowing what is in the Dictionary at any given time. (Is "Foo" a valid key? Has it been set yet? Is it spelled correctly? Is it a bool?)

These programs are not OO. The coding style seems to be more like procedural(?). In general there are many (on the order of 100) keys, and 2 to 3 Dictionaries holding different data types.

Is there any reason not to just make these properties? E.g. isFoo = config.Foo;

Is there any way to improve this code without rewriting it completely?

  • 1
    I fully understand your concern from the application programming viewpoint. But where does the configuration come from? Key=Value text file? XML file? Windows Registry? Database? Probably the source lends itself more easily to a Dictionary implementation... – Ralf Kleberhoff Sep 14 '17 at 18:12
  • @RalfKleberhoff The source is essentially a string dictionary that gets parsed into different types. It's similar to a very long list of command-line arguments. – Era Nov 15 '17 at 17:20
3

You can extend the dictionary and have the best of both worlds.

class ConfigDictionary : Dictionary
{
    bool getFoo()
    {
        return getBool("Foo");
    }
}

Now the main program can use the old syntax or use a dedicated, early bound property.

  • 1
    Or make it agnostic getValueAsBoolean(String key). – Laiv Nov 13 '17 at 22:19
  • Since OP said there are 100s of keys, this is not going to scale. Bad idea. (@Laiv has it better in his comment) – user949300 Nov 13 '17 at 23:14
  • The number of methods doesn't really affect performance, so it'll scale just fine. I think what you meant to state is that the OP has a lot of typing ahead of him, which may be true, unless he is using code generation or something similar to generate the class that encapsulates the configuration. – John Wu Nov 13 '17 at 23:17
  • You are really suggesting 100s of getFoo() methods? Along with 100s of testGetFoo() unit tests? Unless this is mission critical or top security related, that seems a lot of busy work. The opposite of DRY is WET: "we enjoy typing". – user949300 Nov 13 '17 at 23:46
  • I'm not; the OP is asking for it. It's no more "WET" than using a strongly typed configuration file, e.g. XML, where each item ends up being a property in a class. – John Wu Nov 14 '17 at 0:32
1

For one, there is no easy way of knowing what is in the Dictionary at any given time.

Why not? if(!config.ContainsKey("Foo")) should work fine.

Is "Foo" a valid key?

For this, you need to have the list of valid keys somewhere. Where you get this meta information from depends heavily on the source of the config and where it is defined. If you say it will help you to define this meta information once and in context of the config class, making it a property will help.

Has it been set yet?

If not, the code will throw a "key not found" exception at run time, just as config.Foo will probably throw an exception when the property Foo was not initialized.

Is it spelled correctly?

A property Foo, with the correct spelling once where it is defined, can help here, indeed, to avoid misspellings at other places in code.

Is it a bool?

Again, that is what a property can help you with.

Is there any reason not to just make these properties?

There can be reasons, and you have to check if they apply to your case. Adding properties means you will need an individual class for each type of configuration. That makes it harder to have generalized code which works on different types of configurations. Moreover, if there is code which creates the key strings dynamically (for example, but concatenating two other strings together), this cannot just be directly mapped to properties.

This might be a problem or not for your case, you have to check it.

  • Having to check for existence or catch an Exception is tedious. I prefer returning a default value for almost all cases. (Sometime I use a getRequiredValue() method). Can you elaborate on your reasoning? – user949300 Nov 13 '17 at 23:36
  • @user949300: What is your default value for bool? false? How do you distinguish that default from an actual false setting? – Robert Harvey Nov 14 '17 at 0:33
  • @Robert Harvey Yes, bool is awkward. In the primitive case, you have to make a decision about what is the proper default. At that point, do you care if it is false because the user explicitly said false, or it is false because the user said nothing? ("If you choose not to decide, you still have made a choice"). Does UNIX blow up if you type "ls" instead of "ls ."? If for some reason it is important to differentiate, yes, it's a problem. You need something with a "null" (in Java, a Boolean instead of a boolean), or use a getRequiredValue() type of call with no default. – user949300 Nov 14 '17 at 2:32
  • ... even int can be awkward if there is no obvious "not-set" option like 0 or -1. Floating point has NaN. – user949300 Nov 14 '17 at 2:33
0

In cases like this, I think the best solution is something like:

bool getBool(String key, bool defaultValue);
int getInt(String key, int defaultValue);

etc...

This covers your concerns about "it's not there". If misspelling is a concern, your keys could be enums.

Added In response to @Robert Harvey's comments, yes, this requires that you decide "what is the default". (or use a type that can be null, e.g. in Java Integer instead of int) But so do the other two solutions presented. They will require a lot more boilerplate checking of the results. If the property is absent, ultimately you must either fall back to some default, or fail / throw an Exception. I think it's good to be explicit, and force the designer to think of the reasonable defaults.

Depending on your language you might be able to take advantage of generics or similar. For example, in Java, for some types, you could use:

<T> getConfig(String key, T defaultValue);

For an absolutely essential configuration setting, you can add a getRequiredValue(key) method which throws an Exception.

** Added ** Alternatively, you could pre-fill in your Dictionary with default values, and then Doc's or John's solutions work. The advantage (or disadvantage) of mine is that your fallback default value can vary dynamically.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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