Geek asks and answers

Refactoring large, if more code

I have this very long if still code

 if (errorNumbers.Length == 6)
      {
        if (errorNumbers.Substring(0,4).Equals("1101") || errorNumbers.Substring(0,4).Equals("2121"))
        {
          retStr = "AutoRepickAfterPickError";
        }
        else if (errorNumbers.Substring(0, 4).Equals("1301") || errorNumbers.Substring(0, 4).Equals("2321"))
        {
          retStr = "AutoRepickAfterLAlignError";
        }
        else if (errorNumbers.Substring(0, 4).Equals("1401") || errorNumbers.Substring(0, 4).Equals("2221"))
        {
          retStr = "AutoRepickAfterCAlignError";
        }
        else if (errorNumbers.Substring(0, 4).Equals("1501") || errorNumbers.Substring(0, 4).Equals("2041"))
        {
          retStr = "AutoRepicksAfterManualRecovery";
        }

etc.....

How can I rewrite it to something more pleasant. Trying to learn something new here, and I'm in .net 2.0. Thanks for the help.

+3
source share
7 answers

I displayed error codes in a dictionary like this:

string errorCode = errorNumbers.Substring(0, 4);
Dictionary<string, string> map = new Dictionary<string,string>();
map.Add("1501", "AutoRepicksAfterManualRecovery");
map.Add("2041", "AutoRepicksAfterManualRecovery");
map.Add("1401", "AutoRepickAfterCAlignError");
map.Add("2221", "AutoRepickAfterCAlignError");
map.Add("1301", "AutoRepickAfterPickError");
map.Add("2121", "AutoRepickAfterPickError");
// etc

if(!map.ContainsKey(errorCode))
 throw new Exception("Invalid error code");

return map[errorCode];
+13
source

It should be pretty simple.

if (errorNumbers.Length == 6)
{
    string errNo = errorNumbers.Substring(0, 4);

    switch (errNo)
    {
        case "1101":
        case "2121":
            retStr = "AutoRepickAfterPickError";
            break;
        case "1301":
        case "2321":
            retStr = "AutoRepickAfterLAlignError";
            break;
        case "1401":
        case "2221":
            retStr = "AutoRepickAfterCAlignError";
            break;
        case "1501":
        case "2041":
            retStr = "AutoRepicksAfterManualRecovery";
            break;
    }
}

Also note that unlike Java, in C # you can compare strings with ==.

"123" == "456"does the same thing as "123".Equals("456").

+11
source

, DRY - . errorNumbers.Substring(0, 4) :

string subNumbers = errorNumbers.SubString(0, 4);
if (subNumbers == "1401") // ...

( ):

switch :

switch(subNumbers)
{
    case "1401":
    case "2221":
      retStr = "AutoRepickAfterCAlignError";
      break;
    case "1501":
    case "2041":
      retStr = "AutoRepicksAfterManualRecovery";
      break;
    // etc
}

switch :

var selections = new Dictionary<string, string>()
{
    { "1401", "AutoRepickAfterCAlignError" },
    { "2221", "AutoRepickAfterCAlignError" },
    { "1501", "AutoRepicksAfterManualRecovery" },
    { "2041", "AutoRepicksAfterManualRecovery" },
    // etc
};

if (selections.ContainsKey(subNumbers))
    retStr = selections[subNumbers];

:

, , , if/else. , , , , if/else.

+2
source

You must initialize the dictionary with refferencies betwen errorCode and errorMessage

Dictionary<int, string> errorsCache = new Dictionary<int, string>()
{
  {1101,"AutoRepickAfterPickError"},
  {2121,"AutoRepickAfterPickError"},
  {1401,"AutoRepickAfterCAlignError"},
  ...
}

You can define the GetErrorDescription method

public string GetErrorDescription(string errorNumbers)
{
  string retStr;
  if (errorNumbers.Length == 6)
  {
    int errorCode;  
    if(int.TryParse(errorNumbers.Substring(0,4), out errorCode))
    {
      errorsCache.TryGetValue(errorCode, out retStr);
    }
  }

  return retString;
  // or you can return empty string instead of null
  return retString ?? "";
}
+2
source

First save errorNumbers.Substring(0,4)in a local variable before the long if / else.

Then you can exclude the whole if / else using the map / dictionary from the expected error numbers in the corresponding return lines.

+1
source

You can try the following:

if (errorNumbers.Length == 6)
{

    //Makes it easier to change error codes, as theyre all in one place
    string[] AutoRepickAfterPickError = { "1101", "2121"};
    string[] AutoRepickAfterLAlignError = { "1301", "2321"};
    string[] AutoRepickAfterCAlignError = { "1401", "2221"};
    string[] AutoRepicksAfterManualRecovery = { "1501", "2041"};
    string subString = errorNumbers.Substring(0,4);

        if (AutoRepickAfterPickError.Contains(subString));
        {
          retStr = "AutoRepickAfterPickError";
        }
        else if (AutoRepickAfterLAlignError.Contains(subString))
        {
          retStr = "AutoRepickAfterLAlignError";
        }
        else if (AutoRepickAfterCAlignError.Contains(subString))
        {
          retStr = "AutoRepickAfterCAlignError";
        }
        else if (AutoRepicksAfterManualRecovery.Contains(subString))
        {
          retStr = "AutoRepicksAfterManualRecovery";
        }
}
0
source

What about Select-Case ?: D

errorNumber = errorNumbers.Substring(0,4)
Select Case errorNumber 
   Case 1101,2121
      retStr = "AutoRepickAfterPickError";
   Case 1301,2321
      retStr = "AutoRepickAfterLAlignError";
   ...
End Select
-2
source

More articles:


All Articles