7
\$\begingroup\$

I have a static class:

public static class ConfigurationDetails
{
    public static string SharepointServer { get; set; }
    public static string Username { get; set; }
    public static string Password { get; set; }
    public static string SharepointDomain { get; set; }
    public static string ServiceLibrary { get; set; }
    public static string SqlServer { get; set; }
    public static string SQLUsername { get; set; }
    public static string SQLPassword { get; set; }
    public static string SQLDomain { get; set; }
}

which needs to be updated from the values of a dictionary.

Using if the code is below:

var configValues = ReadConfiguration();

if (configValues.ContainsKey("SharepointServer"))
{
    ConfigurationDetails.SharepointServer = configValues["SharepointServer"];

}
if (configValues.ContainsKey("ServiceLibrary"))
{
    ConfigurationDetails.ServiceLibrary = configValues["ServiceLibrary"];

}
if (configValues.ContainsKey("Username"))
{
    ConfigurationDetails.Username = configValues["Username"];

}
if (configValues.ContainsKey("Password"))
{
    ConfigurationDetails.Password= configValues["Password"];

}

Now this will do the job, but there would be a lot of ifs. I've tried with LINQ, but this is the closest I've gotten:

configValues.Where(kv => kv.Key == "SharepointServer").ToList().ForEach(kv => ConfigurationDetails.SharepointServer = kv.Value);

and it's not doing any good. Any other ideas, please?

\$\endgroup\$
  • 1
    \$\begingroup\$ As a side-note: I wouldn't make configuration a static class \$\endgroup\$ – CodesInChaos Nov 5 '13 at 16:21
  • \$\begingroup\$ How much control do you have over the ReadConfiguration method? Could it be refactored to return a ConfigurationDetails instance rather than a dictionary? \$\endgroup\$ – Dan Lyons Nov 5 '13 at 18:14
10
\$\begingroup\$

One solution would be to encapsulate the logic into a method. A problem with that is that there is no simple way to pass a property to the method.

If the case where the key is not in the dictionary means that the value is null, then you could write an extension method for IDictionary that returns the value, if it's present, or null:

public static string GetValueOrDefault(this IDictionary<string, string> dict, string key)
{
    string result;
    dict.TryGetValue(key, out result);
    return result;
}

You would then use it like this:

ConfigurationDetails.SharepointServer = configValues.GetValueOrDefault("SharepointServer");

Another option (assuming the names of properties and keys always match) would be to use reflection to set all the properties at the same time. Something like:

var properties = typeof(ConfigurationDetails).Getproperties();
foreach (var property in properties)
{
    string value;
    if (configValues.TryGetValue(property.Name, out value))
    {
        property.SetValue(null, value);
    }
}
\$\endgroup\$
3
\$\begingroup\$

First off: I think, LINQ is of little help.

  1. You could do yourself a favour and change your configuration to a Dictionary and the problem would not occur.

  2. You could do it with the ternary operator:

    ConfigurationDetails.SharepointServer=(configValues.ContainsKey("SharepointServer"))?configValues["SharepointServer"]:default;
    

    To make things easier, you could wrap it up into a method like svick's solution

  3. You could do it like svick

  4. You could do it automagically, if you drop the idea of a static class, which is by the way nasty, and deal with a configuration object. then, you could leverage reflection to set the according fields, depending on the keys in your dictionary.

\$\endgroup\$
  • \$\begingroup\$ Oooh automagic reflection... I like :) and +1 for static class being nasty! \$\endgroup\$ – Mathieu Guindon Oct 29 '13 at 15:46
3
\$\begingroup\$

I would carefully think about the class design.

Do you really want your configuration values to be writable from outside the ConfigurationDetails class?

Is ReadConfiguration a really expensive method? If not, why should you read the configuration once instead of reading it every time when user requests it? [In the current design, how would user force a refresh of configuration?]

Do you really want to return NULL for setting value when it's not configured? I prefer to throw a helpful exception which can help me troubleshoot the problem just in case I forgot to specify a key or mistyped it.

I can not answer the above questions for you but this is what you should think about.

Assuming that ReadConfiguration method is cheap, here's how I would write the ConfigurationDetails class.

public static class ConfigurationDetails
{
    public static string SharepointServer {  get { return GetConfigValue('SharepointServer'; }

    public static string Username {  get { return GetConfigValue("Username"); }

    public static string Password {  get { return GetConfigValue("Password"); }

    /* Fill in the rest */

    private static string GetConfigValue(string key)
    {
        var configValues = ReadConfiguration();

        if(! configValues.ContainsKey(key))
            throw new ConfigurationErrorsException("Could not find configuration for key " + key);

        return configValues[key];
    }
}
\$\endgroup\$
  • \$\begingroup\$ Alternatively, if the configuration changes infrequently (or not at all), you could change ConfigurationDetails to be an instance class and use a Lazy<T> field for the configuration dictionary. In the case where there are infrequent changes, you can have a method to reset (i.e., re-assign) the field. \$\endgroup\$ – Dan Lyons Nov 5 '13 at 18:16
2
\$\begingroup\$

I know it would not be worth answering this question, but I would like to suggest that using a dictionary and expecting optimization would not be a good idea.

I have tried to refactor my conditions from if/else or switch to dictionary, and you won't believe that it has cost me more resources than a simple if/else.

So you could try to resolve this issue by using Strategy Patterns. That can help you out for such issues.

\$\endgroup\$
0
\$\begingroup\$

Compressed code could look like:

string dictValue;
ConfigurationDetails.SharepointServer = 
    configValues.TryGetValue("SharepointServer", out dictValue) 
    ? dictValue : null;

TryGetValue also sets dictValue to null if it fails, thus no init required on declaration.

Anyway, I would not like a static class, but pass the dictionary to a constructor or Configure(IDictionary ...) method, and also validate the input before taking it into the config.

\$\endgroup\$

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.