Optimizing multiple if-else from static class with LINQ

Question

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?

Answer 1

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);
    }
}

Answer 2

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.

Answer 3

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];
    }
}

Answer 4

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.

Answer 5

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.