0

Im trying to clean up my code and at the same time avoid reducing performance. I have the following for-each loop id like to clean up:

Dictionary<string, string> algorithmsDictionary = GetAlgorithmsDictionary();
foreach (KeyValuePair<string, string> item in algorithmsDictionary) 
   {
       string hashValue = _hash.CalculateHash(data, item.Value);
       CreateTextBox(item.Key).Text = hashValue;
   }

My thoughts are to remove algorithmsDictionary assignment and just use GetAlgorithmsDictionary() inside the foreach signature:

foreach (KeyValuePair<string, string> item in GetAlgorithmsDictionary()) 
   {
       string hashValue = _hash.CalculateHash(data, item.Value);
       CreateTextBox(item.Key).Text = hashValue;
   }

As you can see the latter looks more clean without the algorithmsDictionary assignment but is there a performance cost or bad practice using a method call inside a for-each signature?

Bailey
  • 1
  • 1
  • By the way, it's not a bad practice, no multiple calls, both codes are equivalent. You can also just write `CreateTextBox(item.Key).Text = _hash.CalculateHash(data, item.Value);` – Orace Jan 17 '20 at 17:20
  • The scope of the dictionary in the second code sample is limited to the `foreach` block (and limiting scope is a good practice). Otherwise the two samples are equivalent. – Rufus L Jan 17 '20 at 17:34
  • Non-opinion based part of the question is already answered by linked duplicate, the rest is strictly coding style question (whether to have separate variable to get enumerable for iteration or not) and off-topic on SO. – Alexei Levenkov Jan 17 '20 at 17:38

0 Answers0