如何降低 if-else 语句中的圈复杂度答案

【问题标题】：How to reduce Cyclomatic complexity in an if-else statement如何降低 if-else 语句中的圈复杂度
【发布时间】：2012-06-26 13:37:06
【问题描述】：

在这种情况下你会怎么做来降低圈复杂度

if (Name.Text == string.Empty)
    Name.Background = Brushes.LightSteelBlue;

else if(Age.Text == string.Empty)
    Age.Background = Brushes.LightSteelBlue;

else if(...)
    ...

else
{
    // TODO - something else
}

假设我有 30 个或更多。

【问题讨论】：

我认为降低圈复杂度本身不应该是一个目标。编写可读的代码应该。
姓名、年龄等是多态的，还是共享一个有用的基类？
你确定这是你想要的吗？您只想设置一种背景颜色吗？或者您是否希望为每个项设置背景是否为空？
@NominSim 这只是一个例子，我只是在问这个问题，以了解其他人是如何编写代码的。
我第二个 NominSim。如果一开始有几个错误的字段，一次突出显示一个字段会让用户抓狂。

标签： c# cyclomatic-complexity

【解决方案1】：

看起来您对每个“文本框”执行相同的逻辑（至少我认为它们是文本框）。我建议将它们全部放入一个集合并执行以下逻辑：

// Using var, since I don't know what class Name and Age actually are
// I am assuming that they are most likely actually the same class
// and at least share a base class with .Text and .BackGround
foreach(var textBox in textBoxes)
{
    // Could use textBox.Text.Length > 0 here as well for performance
    if(textBox.Text == string.Empty)
    {
        textBox.Background = Brushes.LightSteelBlue;
    }
}

注意：这确实会稍微改变您的代码，因为我注意到您只有在前一个没有空文本的情况下才检查一个“TextBox”的值。如果你想保持这个逻辑，只需在textBox.Background = Brushes.LightSteelBlue; 后面加上一个break; 语句，只有第一个空的“TextBox”才会设置它的背景颜色。

【讨论】：

这与 OPs 代码执行的功能不同。
@NominSim 如果你在if 中添加break 会这样。
@svick 它会很接近，你将不得不添加某种 else 子句来捕捉最后一种情况......我将切换我的 -1 无论如何，因为 OP 现在已经指定这只是一个例子。
我一发布代码就注意到了这一点。当你发表评论时，我正在编辑中。我将其保留原样，因为我想 OP 实际上并不想一次只突出显示一个“TextBox”，但我在底部添加了一条注释，说明如果他们确实需要建议的修复。跨度>

【解决方案2】：

例如对于这个具体的案例，你可以

定义一个Dictionary<string, dynamic> dic，其中 KEY 是 字符串值，VALUE 是动态的(Name, Age.. .whatever)
做 dic[stringValue].Background = Color.LightSteelBlue;

只是一个例子。

您可能想选择dynamic或不选择。可能是更直观、更容易理解的东西，但基本思想是：

使用基于if 的right-value 键的字典，并喜欢某个操作/方法/对象的值。

希望这会有所帮助。

【讨论】：

每次有人使用dynamic 不是因为互操作，而是因为他想用动态语言编写代码，小猫死在某个地方；）说真的，如果你想使用强类型语言，为什么要使用强类型语言做动态的东西？我不知何故不喜欢它...
@PhilipDaubmeier：就像张开翅膀从山上跳下来一样。如果一个人不会飞，就不要跳。事实上，我解释了解决方案背后的基本思想，并给出了一些自定义实现，可能或可能不被选择。
是的，我明白你的意思。我只是想指出，人们在使用dynamic之前应该考虑他们是否真的需要。

【解决方案3】：

我完全同意 svick 的评论。在某些情况下，以下方法可能很好（但不会降低圈复杂度，通常是为了创建可插入的决策者）：

public class SwitchAction{
  public Func<bool> Predicate { get; set; }
  public Action TheAction { get; set; }
}

public List<SwitchAction> SwitchableActions = new List<SwitchAction>();

public void InitialiseSwitchableActions()
{
   SwitchableActions.AddRange(new[] {
     new SwitchAction() { Predicate = () => Name.Text == string.Empty, 
                          TheAction = () => Name.Background = Brushes.LightSteelBlue },
     new SwitchAction() { Predicate = () => Age.Text == string.Empty, 
                          TheAction = () => Age.Background = Brushes.LightSteelBlue },
   });
}

public void RunSwitchables()
{
  var switched = SwitchableActions.FirstOrDefault(s => Predicate());

  if(switched != null)
    switched.TheAction();
  else
    //TODO: something else.
}

当然 - 如果实际上这些操作不是相互排斥的，您必须稍微更改最后一个方法：

public void RunSwitchables()
{
   bool runCatchAll = true;
   foreach(var switched in SwitchableActions.Where(a => a.Predicate())
   {
     switched.TheAction();
     runCatchAll = false;
   }

   if(runCatchAll)
     //TODO: Something else.
}

这些中的任何一个都更具可读性吗？嗯……可能不会。

【讨论】：