0
\$\begingroup\$

I have a dictionary that I need to iterate efficiently. I already know about the keys beforehand which I need to extract from values dictionary object. What's the best way to do that? I have around 20 entries in dictionary object as of now.

Below is how I am doing it but wanted to see if we can optimize it by just getting values of key directly since I already know about what key I need to extract from them instead of using foreach loop here which is not good in terms of performance?

  // predefined keys
  public const string ABC = "header-abc";
  public const string PQR = "header-pqr";
  public const string DEF = "header-def";
  public const string HIP = "header-hip";

  foreach (KeyValuePair<string, StringValues> v in values)
  {
      string key = v.Key;
      StringValues val = v.Value;
      if (val.Count > 0)
      {
          if (!string.IsNullOrWhiteSpace(val[0]))
          {
              switch (key)
              {
                  case ABC:
                      HeadOne = val[0];
                      HeadOneId = HeadOne;
                      break;
                  case PQR:
                      HeadTwo = val[0];
                      HeadTwoId = HeadTwo;
                      break;
                  case DEF:
                      HeadThree = val[0];
                      HeadThreeId = HeadThree;
                      break;
                  case HIP:
                      HeadFour = val[0];
                      HeadFourId = HeadFour;
                      break;
                  //.. bunch of other case block here with similar stuff
              }
          }
      }
  }

As of now it looks redundant to iterate values dictionary object since I already know about all my keys that I need to extract from values dictionary object. What's the best way (in terms of performance) to do this considering keys are already known beforehand?

One option I can think of is instead of foreach loop here can we use keys property of iterating dictionary?

\$\endgroup\$
3
  • 4
    \$\begingroup\$ While values is a strange name for a dictionary anyway, you don't explicitly state its type. You don't show but a couple of key values to handle; in particular, you don't show how irrelevant keys are handled/ignored. You don't indicate the ratio of keys to handle vs. keys to ignore: There is too much context missing to give a meaningful review. \$\endgroup\$
    – greybeard
    Apr 30, 2020 at 6:21
  • \$\begingroup\$ Why do you do HeadOne = val[0]; and then HeadOneId = HeadOne;? Why would you store the same value in two variables like that? What is even the point of extracting the values from the Dictionary and storing them in variables -- why not use the Dictionary directly? What's the point of having a value in your Dictionary that is of type StringValues when you only are going to use a single value from it? \$\endgroup\$
    – BCdotWEB
    Apr 30, 2020 at 6:55
  • \$\begingroup\$ I agree we're missing something here. Next time, please add the real code and take a look at the help center. \$\endgroup\$
    – Mast
    May 1, 2020 at 19:11

2 Answers 2

3
\$\begingroup\$

I believe you can do this if you put the header keys inside an array, and then you can loop over the array itself, and use TryGetValue for the dictionary.

This is the simplest form for it :

public readonly string[] HeaderKeys = {
    "header-abc", 
    "header-pqr", 
    "header-def", 
    "header-hip"
}

for(var x = 0; x < HeaderKeys.Length; x++)
{
    var isKeyExist = values.TryGetValue(HeaderKeys[x], var KeyValue);

    // if there is no key defined, just go to the next iteration.
    if(!isKeyExist) { continue; }   

    switch (HeaderKeys[x])
    {
      case "header-abc":
          // inline assigning
          HeadOne = HeadOneId = KeyValue; 
          break;
      case "header-pqr":
          HeadTwo = HeadTwoId = KeyValue;
          break;
      case "header-def":
          HeadThree = HeadThreeId = KeyValue;
          break;
      case "header-hip":
          HeadFour = HeadFourId = KeyValue;
          break;
      //.. bunch of other case block here with similar stuff
    }
}

You need a readonly string[] this way, it can't be changed once initialized. System.Array is the lightest, and yet the simplest array, so using for loop with it would give you the fastest iteration over any other collection type.

Next, you only want to pass each header key to the TryGetValue and get the value. The TryGetValue would return true if the value has been out, and false if there is none.

Then, you have one line if condition :

    // if there is no key defined, just go to the next iteration.
    if(!isKeyExist) { continue; }   

This would skip any iteration if the result of TryGetValue is false. And I believe that string.IsNullOrWhiteSpace(KeyValue) is an extra unneeded work. So, I got rid of it. because even if the KeyValue is null or empty, the variables would be also null. So, at this stage you're just assigning variables, the other validation would be on other part of the current system where it needs to be critical to have a value.

Now, assigning the variables like this :

HeadOne = KeyValue;
HeadOneId = HeadOne;

can be simplified to this :

HeadOne = HeadOneId = KeyValue;

the difference is that the first one would let the compiler read the value twice. While the second would read it once, and assign it twice.

\$\endgroup\$
1
  • \$\begingroup\$ I have another question which needs code review here. Wanted to see if you have some time to take a look and provide some suggestions and improvements basis on my code. Any help will be appreciated. \$\endgroup\$
    – dragons
    Jul 19, 2020 at 2:05
2
\$\begingroup\$

I'm not sure I understand completely what you're trying to achieve. [Sorry, I'm not allowed to write a comment yet, so I couldn't ask for clarification there...]

If you know what keys you're looking for, you can just get the value in 1 step from the Dictionary.

public const string searchKey = "key2";

Dictionary<string, string> values = new Dictionary<string, string>() {
  { "key1", "value1" },
  { "key2", "value2" },
  { "key3", "value3" }
};

// A: If you are sure the key can be found in the dictionary or you want to get an Exception in case it is not
string v = values[searchKey]; // v will be "value2"

// B: If you're not sure if the key is in the dictionary and you don't want an Exception
if (values.TryGetValue(searchKey, out string val))
{
  // Here val will be "value2"
}
else
{
  // This code runs when searchKey was not among the dictionary keys
}

Of course Option A is faster, but see the comment if it fits your situation. There is no faster way to get a value from a Dictionary.

Based on this, your code would be something like this:

// predefined keys
public const string ABC = "header-abc";
public const string PQR = "header-pqr";
public const string DEF = "header-def";
public const string HIP = "header-hip";

HeadOne = values[ABC];
HeadOneId = HeadOne;

HeadTwo = values[PQR];
HeadTwoId = HeadTwo;

HeadThree = values[DEF];
HeadThreeId = HeadThree;

HeadFour = values[HIP];
HeadFourId = HeadFour;

or with option B:

// predefined keys
public const string ABC = "header-abc";
public const string PQR = "header-pqr";
public const string DEF = "header-def";
public const string HIP = "header-hip";

if (values.TryGetValue(ABC, out string val))
{
  HeadOne = val;
  HeadOneId = HeadOne;
}

if (values.TryGetValue(PQR, out string val))
{
  HeadTwo = val;
  HeadTwoId = HeadTwo;
}

if (values.TryGetValue(DEF, out string val))
{
  HeadThree = val;
  HeadThreeId = HeadThree;
}

if (values.TryGetValue(HIP, out string val))
{
  HeadFour = val;
  HeadFourId = HeadFour;
}

I hope this is an answer to your question. If not, please clarify your goal or ask questions. Thanks!


Update 1 - Answer to your comment:

Yes, totally. If you have the relevant keys in a collection you can iterate through them and use TryGetValue in the loop. But in this case the variable names like "HeadOne", "HeadTwo" or "HeadFourId" need to be generalized as well. Or you need an extra switch-case inside the loop again, which leads back to square one.

Also, consider the comments on the question, they have some good points, like using the dictionary directly where you need it rather than getting out the values separately and use them later.

An example for the loop:

string[] keys = new string[] { "header-abc", "header-pqr", "header-def", "header-hip" }

IList<string> headerValues = new List<string>();

foreach (var key in keys)
{
  if (values.TryGetValue(key, out string val))
  {
    headerValues.Add(val);
  }
}
\$\endgroup\$
2
  • \$\begingroup\$ thanks for your suggestion. how about using for loop here to iterate all the keys and then using TryGetValue on them instead of having each separate line with if check? maybe I can make separate arrays list here with all the keys and iterate that array? \$\endgroup\$
    – dragons
    Apr 30, 2020 at 2:09
  • \$\begingroup\$ See the update in the answer. \$\endgroup\$
    – Akos
    Apr 30, 2020 at 11:27

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