4

前几天我在代码审查中遇到了一些看起来像这样的代码。

public void DoSomeTasks()
{
  if (CheckSomeState()==true) return;
    DoTaskOne();
  if (CheckSomeState()==true) return;
    DoTaskTwo();
  if (CheckSomeState()==true) return;
    DoTaskThree();
  if (CheckSomeState()==true) return;
    DoTaskFour(); 
}

随着任务数量的增加，代码的圈复杂度越来越高，而且我也觉得不合适。

我想出的解决方案是。

private void DoTasksWhile(Func<bool> condition, Action[] tasks)
{
   foreach (var task in tasks)
   {
      if (condition.Invoke()==false) break;
        task.Invoke();
   }
}

像这样使用

public void DoSomeTasks()
{
 var tasks = new Action[] { 
  {()=DoTaskOne()},
  {()=DoTaskTwo()},
  {()=DoTaskThree()},
  {()=DoTaskFour()}

  }

  DoTasksWhile(()=>CheckSomeState(), tasks);
}

有人有任何建议可以使代码更具可读性吗？

4

3 回答 3

4

我对您的实现进行了一些重构

private void DoTasksWhile(Func<bool> predicate, IEnumerable<Action> tasks)
{
    foreach (var task in tasks)
    {
        if (!predicate())
            return;
        task();
    }
}
  • 你不需要Invoke代表。执行它们
  • 不要将布尔值与true/false（它没用，您可以错误地分配布尔值）
  • 因此你只枚举任务，IEnumerable有利于参数

您也可以创建扩展方法

public static void DoWhile(this IEnumerable<Action> actions,Func<bool> predicate)
{
    foreach (var action in actions)
    {
        if (!predicate())
            return;
        actions();
    }
}

用法将非常简单：

tasks.DoWhile(() => CheckSomeState());
于 2012-10-27T17:32:11.487 回答
1

如果您的编排代码将非常复杂，请考虑使用诸如 WF（工作流基础）之类的框架。

否则，您的代码很好，但我不会更改最初的代码，因为它更具可读性。它也更加灵活，因为将来您可以修改这些 IF，即条件可能不会保持相同。

我看不到圈复杂度如何降低或可以降低。

于 2012-10-27T17:31:45.683 回答
0

您的解决方案就目前而言是完全足够的（尽管在只有四个步骤的情况下可能有点过分），但是在这种情况下它是否是最好的方法取决于 CheckSomeState 在您的程序上下文中的实际用途：

  • 在每个任务之间只调用一次 CheckSomeState 是否重要？
  • 有时可能还需要在任务中途调用它吗？

例如，如果 CheckSomeState 实际上正在检查取消标志，那么在长时间运行的任务中可能需要额外的检查。

您可以使用的另一个选项是抛出异常（例如 OperationCancelledException）。这使您的圈复杂度非常低，因为您现在可以简单地执行以下操作：

CheckForCancel();
DoTaskOne();
CheckForCancel();
DoTaskTwo();

（缺点是有些人会认为这是对控制流使用异常，这通常是不受欢迎的）。

我还应该补充一点，目标不应该是降低圈复杂度，而是要有易于理解和维护的代码。圈复杂度是一个有用的指标，但它有时会不必要地惩罚非常简单的代码。

于 2012-10-27T17:40:14.603 回答