When to write code to anticipate looping and when to specifically do something 'X' times

StackOverflow https://stackoverflow.com/questions/17572462

  •  02-06-2022
  •  | 
  •  

Domanda

If i know that a method must perform an action a certain amount of times, such as retrieve data, should I write code to specifically do this the number of times that is required, or should my code be able to anticipate later changes? For instance, say I was told to write a method that retrieves 2 values from a dictionary (Ill call it Settings here) and return them using known keys that are provided

public Dictionary<string, string> GetSettings()
{
   const string keyA = "address"; //I understand 'magic strings' are bad, bear with me
   const string keyB = "time"
   Dictionary<string, string> retrievedSettings = new Dictionary<string,string>();

    //should I add the keys to a list and then iterate through the list?

    List<string> listOfKeys = new List<string>(){keyA, keyB};
    foreach( string key in listOfKeys)
    {

      if(Settings.ContainsKey(key)
      {
          string value = Setting[key];
          retrieveSettings.Add(key, value);
      }
    }

    //or should I just get the two values directly from the dictionary like so

    if(Settings.ContainsKey(keyA)
    {
        retrievedSettings.Add(keyA , Setting[keyA]);
    }

    if(Settings.Contains(keyB)
    {
        retrievedSettings.Add(keyB , Setting[keyB]);
    }



    return retrievedSettings   
}

The reason why I ask is that code repetition is always a bad thing ie DRY, but at the same time, more experienced programmers have told me that there is no need write the logic to anticipate larger looping if it the action only needs to be performed a known number of times

È stato utile?

Soluzione 2

The DRY principle does not necessarily mean that every line of code in your program should be unique. It simply means that you should not have large regions of code spread out throughout your program that do the same thing.

Option number 1 works well when you have a large number of items to search for, but has the downside of making the code slightly less trivial to read.

Option number 2 works well when you have a small number options. It is more straightforward and is actually more efficient.

Since you only have two settings, I would definitely go with option number 2. Making decisions such as these in expectation of future changes is a waste of effort. I have found this article to be quite helpful in illustrating the perils of being too concerned with non-existent requirements.

Altri suggerimenti

I would extract a method that takes the keys as parameter:

private Dictionary<string, string> GetSettings(params string[] keys)
{
    var retrievedSettings = new Dictionary<string, string>();
    foreach(string key in keys)
    {
        if(Settings.ContainsKey(key)
            retrieveSettings.Add(key, Setting[key]);
    }
    return retrievedSettings;
}

You can now use this method like this:

public Dictionary<string, string> GetSettings()
{
    return GetSettings(keyA, keyB);
}

I would choose this approach because it makes your main method trivial to understand: "Aha, it gets the settings for keyA and for keyB".
I would use this approach even when I am sure that I will never need to get more than these two keys. In other words, this approach has been chosen, not because it anticipates later changes but because it better communicates intent.


However, with LINQ, you wouldn't really need that extracted method. You could simply use this:

public Dictionary<string, string> GetSettings()
{
    return new [] { keyA, keyB }.Where(x => Settings.ContainsKey(x))
                                .ToDictionary(x => x, Settings[x]);
}
Autorizzato sotto: CC-BY-SA insieme a attribuzione
Non affiliato a StackOverflow
scroll top