Efficient alternative way than using multiple if statements to check keys in a dictionary

Question

I have around 45 if statements that checks the dictionary to see if it has that key.

    private List<MessageName> createMsgObject(Dictionary<string, string> infoHash, Dictionary<string, Int16> msgDescritionAndID)
    {
            MessageName msgName = null;
            List<MessageName> msgNameList = new List<MessageName>();
            var msgObjOuter = new MessageName();

            if (infoHash.ContainsKey("redis_version"))
            {
                msgName = new MessageName();
                msgName.MessageID = msgDescritionAndID["redis_version"];
                msgName.DiagnosticCnt = 0;
                msgName.DiagnosticStr = infoHash["redis_version"]; 
                msgNameList.Add(msgName);
            }
           if (infoHash.ContainsKey("uptime_in_seconds"))
           {
            msgName = new MessageName();
            msgName.MessageID = msgDescritionAndID["uptime_in_seconds"];
            msgName.DiagnosticCnt = Convert.ToInt32(infoHash["uptime_in_seconds"]);
            msgName.DiagnosticStr = "";
            msgNameList.Add(msgName);
          }
          //... 40 more if statements
         return msgNameList;
   }

This really hasn't had a hit on performance but I was wondering if there was a more efficient way to check all the keys in that dictionary and if it does insert it's property in to a MessageName object and insert that object into a list. I don't know if switch statements would help in performance.

Answer 1

There is no need to perform an dictionary lookup for all of your messages, unless your dictionary has a lot of keys you are not interested in, in which case stick with what you have.

Otherwise, You already know what keys are in the dictionary so you can just enumerate them.

For those that message that you are currently looking up the value after checking for the key you will be eliminating another lookup because enumerating the dictionary has the extra benefit of getting the value for free.

Then all that is left to do is lookup the int16 from your second dictionary for those keys that you want to process.

You can process this more efficiently and reduce your code base like this (assuming infoHash has only keys you want):

private List<MessageName> createMsgObject(Dictionary<string, string> infoHash, Dictionary<string, Int16> msgDescritionAndID)
{
    MessageName msgName = null;
    List<MessageName> msgNameList = new List<MessageName>();
    var msgObjOuter = new MessageName();
    // there is no need to perform an o(1) lookup of infoHash for 40 variables. 
    // You already have a list of keys it contains so just enumerate them.
    foreach (KeyValuePair<string, string> info in infoHash)
    {
        var msg = new MessageName() { MessageID = msgDescritionAndID[info.Key] };
        switch (info.Key)
        {
            // switch all of your int16 versions first:
            case "redis_version":
            case "hash_int_message_2":
            case "hash_int_message_3":
                msg.DiagnosticCnt = Convert.ToInt32(infoHash[info.Value]);
                break;
            // switch on all message types getting int16 from info.Key
            case "msg_int_message_1":
            case "msg_int_message_2":
                msg.DiagnosticCnt = Convert.ToInt32(msgDescritionAndID[info.Key]);
                break;
            // everything left over is reading value from our current info.
            //  default:
                msg.DiagnosticStr = info.Value;
                break;

        }
        msgNameList.Add(msgName);
    }
    return msgNameList;
}

Even if infoHash contains keys your not interested in, you can code it like this:

private List<MessageName> createMsgObject(Dictionary<string, string> infoHash, Dictionary<string, Int16> msgDescritionAndID)
{
    MessageName msgName = null;
    List<MessageName> msgNameList = new List<MessageName>();
    var msgObjOuter = new MessageName();
    // there is no need to perform an o(1) lookup of infoHash for 40 variables. 
    // You already have a list of keys it contains so just enumerate them.
    foreach (KeyValuePair<string, string> info in infoHash)
    {
        var msg = new MessageName() { MessageID = msgDescritionAndID[info.Key] };
        switch (info.Key)
        {
            // switch all of your int16 versions first:
            case "redis_version":
            case "hash_int_message_2":
            case "hash_int_message_3":
                msg.DiagnosticCnt = Convert.ToInt32(infoHash[info.Value]);
                break;
            // switch on all message types getting int16 from info.Key
            case "msg_int_message_1":
            case "msg_int_message_2":
                msg.DiagnosticCnt = Convert.ToInt32(msgDescritionAndID[info.Key]);
                break;
            // switch on all message types that have DiagnosticStr in info.Value;
            case "msg_str_message_1":
            case "msg_str_message_2":
                msg.DiagnosticStr = info.Value;

                break;
            default:    // everything left over we are not interested in
                continue;
                break;

        }
        msgNameList.Add(msgName);
    }
    return msgNameList;
}

Answer 2

Switch statements do help in performance, compared to multiple ifs, as discussed here.

However I have a suggestion, if there are multiple occasions in your 40+ statements where you assign the same properties to the same variables, you could try something like:

List<string> toCheckStrings1 = new List<string> { "redis_version", "something similar", };      //you can put all the strings here first

Then with a foreach loop you can reduce the number of if statements

foreach (var match in toCheckStrings1.Intersect(infoHash.Keys))    //intersect() will give you a list of matched keys
{
    msgNameList.Add(new MessageName
    {
        MessageID = msgDescritionAndID[match],
        DiagnosticCnt = 0,
        DiagnosticStr = infoHash[match]
    });
}

And since you assign different properties for some variables (e.g. DiagnosticCnt for "uptime_in_seconds"), you can group those differently:

List<string> toCheckStrings2 = new List<string> { "uptime_in_seconds", "others", "etc" };
foreach (var match in toCheckStrings2.Intersect(infoHash.Keys))
{
    msgNameList.Add(new MessageName
    {
        MessageID = msgDescritionAndID[match],
        DiagnosticCnt = Convert.ToInt32(infoHash["match"]),
        DiagnosticStr = ""
    });
}

And so on for other cases. Even though it may be several foreach loops, hopefully it would be much less than 45 if statements.

EDIT

As pointed out in the comments, the time complexity of my suggestion above would be somewhere around O(N) while for your code it would be a bunch of O(1) which is basically O(1). As such, if time is a concern (if you have huge datasets), it would be wiser to just stick with if statements, or switch.