为什么我要重构这段代码，因为圈复杂度是58

我读到CC 10或更低将是高度可维护的代码。但是我写的方法有cc58。感谢VS 2010代码分析工具。我相信我写的方法是非常简单的，可读性和可维护性就我的理解。因此，我不喜欢重构代码。但是既然CC比可接受的要高，我想知道为什么要重构这个方法。我正在学习的东西，以提高我的代码，如果我有错误，请纠正我。下面是代码。

private string MapBathRooms(string value)
    {
        double retValue = 0;
        if (value == "1" || value == "One")
            retValue = 1;
        if (value == "OneAndHalf" || value == "1.5" || value == "1 1/2")
            retValue = 1.5;
        if (value == "2" || value == "Two")
            retValue = 2;
        if (value == "TwoAndHalf" || value == "2.5" || value == "2 1/2")
            retValue = 2.5;
        if (value == "3" || value == "Three")
            retValue = 3;
        if (value == "ThreeAndHalf" || value == "3.5" || value == "3 1/2")
            retValue = 3.5;
        if (value == "4" || value == "Four")
            retValue = 4;
        if (value == "FourAndHalf" || value == "4.5" || value == "4 1/2")
            retValue = 4.5;
        if (value == "5" || value == "Five" || value == "FourOrMore")
            retValue = 5;
        if (value == "FiveAndHalf" || value == "5.5" || value == "5 1/2")
            retValue = 5.5;
        if (value == "6" || value == "Six")
            retValue = 6;
        if (value == "SixAndHalf" || value == "6.5" || value == "6 1/2")
            retValue = 6.5;
        if (value == "7" || value == "Seven")
            retValue = 7;
        if (value == "SevenAndHalf" || value == "7.5" || value == "7 1/2")
            retValue = 7.5;
        if (value == "8" || value == "8+" || value == "Eight" || value == "SevenOrMore")
            retValue = 8;
        if (value == "EightAndHalf" || value == "8.5" || value == "8 1/2")
            retValue = 8.5;
        if (value == "9" || value == "Nine")
            retValue = 9;
        if (value == "NineAndHalf" || value == "9.5" || value == "9 1/2")
            retValue = 9.5;
        if(value == "10" || value == "Ten")
            retValue = 10;
        if (value == "TenAndHalf" || value == "10.5" || value == "10 1/2"
            || value == "10+" || value == "MoreThanTen" || value == "11")
            retValue = 10.5;
        if (retValue == 0)
            return value;
        return retValue.ToString();
    }

为什么不拥有一个Dictionary<string, double> ?这将使for 的代码更加简单——您已经将数据从查找代码中分离出来了。

private static readonly Dictionary<string, double> BathRoomMap =
    new Dictionary<string, double>
{
    { "1", 1 },
    { "One", 1 },
    { "OneAndHalf", 1.5 },
    { "1.5", 1.5 },
    { "1 1/2", 1.5 }
    // etc
};
private static string MapBathRooms(string value)
{
    double result;
    if (!BathRoomMap.TryGetValue(value, out result))
    {
        return value; // Lookup failed
    }
    return result.ToString();
}

实际上，您可以通过避免ToString调用来使它更简单-只需将其设置为Dictionary<string, string>:

private static readonly Dictionary<string, string> BathRoomMap = new Dictionary<string, string> { // Note: I've removed situations where we'd return the // same value anyway... no need to map "1" to "1" etc { "One", "1" }, { "OneAndHalf", "1.5" }, { "1 1/2", "1.5" } // etc }; private static string MapBathRooms(string value) { string result; if (!BathRoomMap.TryGetValue(value, out result)) { return value; // Lookup failed } return result; }
正如ChrisF所说，您也可以从文件或其他资源中读取它。
这样做的好处:
在我看来，更容易避免错误和扩展。从输入到输出有一个简单的1:1映射，与可能出错的逻辑相反
从逻辑中分离出数据
它允许你从其他地方加载数据，如果需要的话。
因为集合初始化器使用Dictionary<,>.Add，如果你有一个重复的键，你会得到一个异常，当你初始化类型，所以你会立即发现错误。
这么说吧——你会考虑从基于字典的版本重构到"大量真实代码"的版本吗?我当然不会。
如果你真的，真的想在方法中拥有所有这些，你总是可以使用一个switch语句:

private static string MapBathRooms(string value) { switch (value) { case "One": return "1"; case "OneAndHalf": case "1 1/2": return "1.5"; ... default: return value; } }
我自己还是会用字典的形式…但是这确实有一个很小的优点，重复检测被提到了编译的时间。

我同意其他海报关于使用字典进行映射的观点，但我也想指出，这样的代码通常很难找到bug。例如:

您将"FourOrMore"转换为5，但"MoreThanTen"转换为10.5。这似乎不一致。
你把"11"转换成10.5，这似乎也与其他代码不一致。

用于进行转换的通用算法最初可能很难编写，但从长远来看可以很容易地节省您的时间。

回答为什么而不是怎么做:

其中一个原因是我在Jon Skeet的回答评论中提到的，但是使用字典和外部资源允许您修改应用程序的行为，而不必在每次需求更改时重新构建它。

另一个是执行速度。您的代码必须检查几十个字符串才能找到结果——虽然有一些方法可以在找到匹配后停止执行，但您仍然需要检查所有的字符串。使用字典将为您提供线性访问时间，而不考虑输入。

是的。非常可维护。

试试这个:

// initialize this somewhere
IDictionary<string, string> mapping;
private string MapBathRooms(string value)
{
  if (mapping.ContainsKey(value))
  {
    return mapping[value];
  }
  return value;
}

将此保存在字典中应该使CC仅为2。字典可以通过从文件或其他资源中读取来初始化。

CC(几乎)是一个方法的潜在执行路径的数量。这种方法的CC很高，因为您没有使用适合处理这类问题的结构(这里是字典)。使用适当的数据结构来解决问题可以保持代码的整洁和可重用性。

使用DRY原则(不要重复自己)，您可以将所有这些if语句替换为switch。切换将使用哈希表实现，因此它也将比所有if语句更快。

您可以删除捕获数字表示的所有情况，因为这是由回退处理的。

我不认为将字符串转换成数字，然后再转换回字符串有什么意义。使用文字字符串(因为它们是预先创建的)比动态创建字符串更有效。此外，这也消除了区域性问题，例如，对于某些区域性，值9.5将导致字符串"9,5"而不是"9.5"。

private string MapBathRooms(string value) {
  switch (value) {
    case "One": value = "1"; break;
    case "OneAndHalf":
    case "1 1/2": value = "1.5"; break;
    case "Two": value = "2"; break;
    case "TwoAndHalf":
    case "2 1/2": value = "2.5"; break;
    case "Three": value = "3"; break;
    case "ThreeAndHalf":
    case "3 1/2": value = "3.5"; break;
    case "Four": value = "4"; break;
    case "FourAndHalf":
    case "4 1/2": value = "4.5"; break;
    case "Five":
    case "FourOrMore": value = "5"; break;
    case "FiveAndHalf":
    case "5 1/2": value = "5.5"; break;
    case "Six": value = "6"; break;
    case "SixAndHalf":
    case "6 1/2": value = "6.5"; break;
    case "Seven": value = "7"; break;
    case "SevenAndHalf":
    case "7 1/2": value = "7.5"; break;
    case "8+":
    case "Eight":
    case "SevenOrMore": value = "8"; break;
    case "EightAndHalf":
    case "8 1/2": value = "8.5"; break;
    case "Nine": value = "9"; break;
    case "NineAndHalf":
    case "9 1/2": value = "9.5"; break;
    case "Ten": value = "10"; break;
    case "TenAndHalf":
    case "10 1/2":
    case "10+":
    case "MoreThanTen":
    case "11": value = "10.5"; break;
  }
  return value;
}

注意，我把输入"11"的大小写留在了"10.5"的返回值中。我不确定这是否是一个bug，但这就是原始代码所做的。

对于您的一般问题，对于其他无法按照其他响应者对该特定功能的建议进行重构的其他用例，CC有一种变体，它将用例语句计算为单个分支，理由是它实际上与易于理解的线性代码行相同(尽管不是为了测试覆盖率)。许多测量一种变体的工具将提供另一种变体。我建议使用case=1的变体，或者和你正在使用的一样。

相关内容

最新更新

热门标签：