重构庞大的if else代码

Question

重构庞大的if else代码

4

我有一个非常冗长的if else代码

 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";
        }

我该如何将它重写为更好的形式。正在尝试学习新的东西，而且我使用的是 .net 2.0。感谢您的帮助。

- Night Walker

3

首先，DRY原则 - 不要重复自己（http://en.wikipedia.org/wiki/Don't_repeat_yourself）。将`errorNumbers.Substring(0, 4)`的结果分配给一个本地变量。 - Merlyn Morgan-Graham

你“可以”使用策略模式，但在大多数情况下这是完全不必要的，有时也难以维护。 - Kane

7个回答

11

这应该非常简单

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;
    }
}

需要注意的是，与Java不同，在C#中你可以使用==比较字符串。

"123" == "456"与"123".Equals("456")是相同的。

- SirViver

大的IfElse / SwitchCase留在这里。 - Viacheslav Smityukh

2

首先，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 语句块，它们往往会过于复杂。

- Merlyn Morgan-Graham

2

您需要使用errorCode和errorMessage之间的引用来初始化字典。

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

你可以定义GetErrorDescription方法。

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 ?? "";
}

- Viacheslav Smityukh

1

首先，在长长的 if/else 语句之前，将 errorNumbers.Substring(0,4) 存储在一个本地变量中。

然后，您可以使用从预期错误编号到相应返回字符串的映射/字典来消除整个 if/else。

- Péter Török

0

你可以试试这个：

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";
        }
}

- strvanica

-2

选择语句怎么样？ :D

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

- Andreas

这是一个关于C#而不是VB.Net的问题。 - Richard Schneider

也许更好的做法是删除答案。 - Kiquenet

网页内容由stack overflow 提供, 点击上面的

可以查看英文原文，
原文链接

- Almund · Accepted Answer

我会像这样将错误码映射到字典中：

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];