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

Question

我读到，拥有 CC 10 或更少的代码将是高度可维护的。但我写的方法有CC 58。感谢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();
    }

Answer 1

为什么不直接使用 Dictionary 呢？这将使代码变得更加简单 - 您已将数据与查找代码分开。

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：

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

我自己仍然使用字典形式......但这确实有一个非常小的优势，即重复检测被提前到 < em>编译时。

Answer 2

我同意其他发帖者关于使用字典进行映射的观点，但我也想指出，这样的代码通常很难发现错误。例如：

• 您将“FourOrMore”转换为 5，但“MoreThanTen”转换为 10.5。这看起来不一致。
• 您将“11”转换为 10.5，这似乎也与代码的其余部分不一致。

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

Answer 3

回答为什么而不是如何：

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

另一个是执行速度。您的代码必须检查几十个字符串才能找到结果 - 虽然有多种方法可以在找到匹配项后停止执行，但您仍然需要检查所有字符串。无论输入如何，使用字典都会为您提供线性访问时间。

Answer 4

是的。确实非常易于维护。

尝试这样做：

// 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那么高，因为你还没有使用适合处理此类问题的结构（这里：字典）。使用适当的数据结构来解决问题可以保持代码整洁且可重用。

Answer 5

根据 DRY（不要重复）的原则，您可以用 switch 替换所有这些 if 语句。该开关将使用哈希表实现，因此它也将比所有 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"。我不确定这是否是一个错误，但这就是原始代码的作用。

Answer 6

对于您的一般问题，对于无法按照其他响应者针对此特定功能的建议进行重构的其他情况，CC 有一种变体，它将案例陈述视为单个分支，因为它实际上与线性行相同代码易于理解（尽管不适用于测试覆盖率）。许多测量一种变体的工具也会提供另一种变体。我建议使用 case=1 变体，或者与您正在使用的变体一起使用。