我怎样才能降低圈复杂度？答案

【问题标题】：how could I reduce the cyclomatic complexity?我怎样才能降低圈复杂度？
【发布时间】：2013-07-29 11:12:46
【问题描述】：

每当我对正在处理的一段代码进行 lint 处理时，我都会得到 This function's cyclomatic complexity is too high. (7)。但我有点困惑如何以这种方式重写它以使其正常工作。

这将是不断抛出该消息的函数：

function () {
  var duration = +new Date() - start.time,
    isPastHalf = Number(duration) < 250 && Math.abs(delta.x) > 20 || Math.abs(delta.x) > viewport / 2,
    direction = delta.x < 0;

  if (!isScrolling) {
    if (isPastHalf) {
      if (direction) {
        this.close();
      } else {
        if (this.content.getBoundingClientRect().left > viewport / 2 && pulled === true) {
          this.close();
          return;
        }
        this.open();
      }
    } else {
      if (this.content.getBoundingClientRect().left > viewport / 2) {
        if (this.isEmpty(delta) || delta.x > 0) {
          this.close();
          return;
        }
        this.open();
        return;
      }
      this.close();
    }
  }
}

我想听听一些关于如何以这种方式构造我的代码以避免这种情况的建议。

【问题讨论】：

是什么导致了这个错误？
认真的吗？不要使用嵌套的ifs。根据单一职责原则分解职责。一段代码（一个模块）应该只做一件事，而且它应该做得很好。想想这些丑陋的 if-bushes 生成了多少可能的执行路径......
你读过代码@Powerslave吗？这如何破坏 SRP？
一些谷歌搜索显示这是特定于jshint，因此您应该相应地标记您的问题。
您可以考虑在codereview.stackexchange.com上发布此类问题

标签： javascript jshint cyclomatic-complexity

【解决方案1】：

您的代码中只有两个操作，但条件太多。在条件中使用单个 if-else 语句和布尔运算符。如果那是不可能的，你至少可以

删除空行以获得一个屏幕页面上的完整逻辑
添加一些关于分支正在做什么（以及为什么）的 cmets
避免提前退货

这是您的简化功能：

var duration = +new Date() - start.time,
    isPastHalf = Number(duration) < 250 && Math.abs(delta.x) > 20 || Math.abs(delta.x) > viewport / 2,
    isFarRight = this.content.getBoundingClientRect().left > viewport / 2, 
    direction = delta.x < 0;

if (!isScrolling) {
    if (isPastHalf) {
        if (direction)
            this.close();
        else {
            if (isFarRight && pulled)
                this.close();
            else
                this.open();
        }
    } else {
        if (isFarRight) {
            // Looks like the opposite of `direction`, is it?
            if (this.isEmpty(delta) || delta.x > 0)
                this.close();
            else
                this.open();
        } else
            this.close();
    }
}

并缩短：

var duration = +new Date() - start.time,
    isPastHalf = Number(duration) < 250 && Math.abs(delta.x) > 20 || Math.abs(delta.x) > viewport / 2,
    isFarRight = this.content.getBoundingClientRect().left > viewport / 2, 
    direction = delta.x < 0,
    undirection = this.isEmpty(delta) || delta.x > 0;

if (!isScrolling) {
    if ( isPastHalf && !  direction && !(isFarRight && pulled)
     || !isPastHalf && !undirection &&  isFarRight )
        this.open();
    else
        this.close();
}

【讨论】：

switch 语句的圈复杂度怎么样，如果我有类似 this 的东西，我该怎么办？我认为我无能为力，或者我可以吗？
您可以做很多事情。首先，当您不需要 falltrough 时，通常会使用 else-ifs 而不是 switch。
顺便说一句，如果你在尝试最小化布尔数学时脑残，请作弊并使用wolframalpha
@Peri：避免提前返回确实使代码更清晰。
iscroling 的保护返回甚至可以在分配之前完成。此外，if 对我的口味来说太复杂了。我将引入另外两个变量来揭示 isPastHalf && !direction && !(isFarRight && pull) 和 !isPastHalf && direction && isFarRight 的实际含义。

【解决方案2】：

首先，您的函数可以有三种结果：什么都不做，调用this.close() 或调用this.open()。因此，理想情况下，生成的函数将只有一个 if 语句来确定使用哪个结果。

下一步是将所有布尔代码提取到变量中。例如var leftPastCenter = this.content.getBoundingClientRect().left > viewport / 2。

最后，使用布尔逻辑逐步简化。

我是这样做的：

首先，提取所有布尔变量：

function () {
    var duration = +new Date() - start.time,
      isPastHalf = Number(duration) < 250 && Math.abs(delta.x) > 20 || Math.abs(delta.x) > viewport / 2,
      direction = delta.x < 0,
      leftPastCenter = this.content.getBoundingClientRect().left > viewport / 2,
      positiveDelta = this.isEmpty(delta) || delta.x > 0,
      isPulled = pulled === true; // I'll assume the test is needed rather than just using pulled.

    if (!isScrolling) {
        if (isPastHalf) {
            if (direction) {
                this.close();
            } else {
                if (leftPastCenter && isPulled) {
                    this.close();
                    return;
                }
                this.open();
            }
        } else {
            if (leftPastCenter) {
                if (positiveDelta) {
                    this.close();
                    return;
                }
                this.open();
                return;
            }
            this.close();
        }
    }
}

最容易摆脱的部分是意识到如果isScrolling 为真，那么什么都不会发生。这立即摆脱了一层嵌套：

    // above same
    if (isScrolling) { return; }

    if (isPastHalf) {
        if (direction) {
            this.close();
        } else {
            if (leftPastCenter && isPulled) {
                this.close();
                return;
            }
            this.open();
        }
    } else {
        if (leftPastCenter) {
            if (positiveDelta) {
                this.close();
                return;
            }
            this.open();
            return;
        }
        this.close();
    }
}

现在看看this.open() 被调用的案例。如果isPastHalf 为真，则this.open() 仅在!direction 和!(leftPastCenter && isPulled) 时被调用。如果isPastHalf 为假，则this.open() 仅在leftPastCenter 和!positiveDelta 时被调用：

    // above same
    if (isScrolling) { return; }

    if (isPastHalf) {
        if (!direction && !(leftPastCenter && isPulled)) {
            this.open();
        } else {
            this.close();
        }
    } else {
        if (leftPastCenter && !positiveDelta) {
            this.open();
        } else {
            this.close();
        }
    }

翻转 ifs（所以 this.close() 先出现），使代码更整洁，并给出我的最终版本：

    function () {

    var duration = +new Date() - start.time,
      isPastHalf = Number(duration) < 250 && Math.abs(delta.x) > 20 || Math.abs(delta.x) > viewport / 2,
      direction = delta.x < 0,
      leftPastCenter = this.content.getBoundingClientRect().left > viewport / 2,
      positiveDelta = this.isEmpty(delta) || delta.x > 0,
      isPulled = pulled === true; // I'll assume the test is needed rather than just using pulled.

    if (isScrolling) { return; }

    if (isPastHalf) {
        if (direction || (leftPastCenter && isPulled)) {
            this.close();
        } else {
            this.open();
        }
    } else {
        if (!leftPastCenter || positiveDelta) {
            this.close();
        } else {
            this.open();
        }
    }
}

在不了解您的代码库的情况下，我很难做得更多。需要注意的一点是direction 和我的新变量positiveDelta 几乎相同——您可以删除positiveDelta 并只使用direction。此外，direction 不是布尔值的好名字，像movingLeft 这样的名字会更好。

【讨论】：

【解决方案3】：

实际上，所有这些return 声明都在混淆问题，但它们提供了解决方案的提示。

if (direction) {
  this.close();
} else {
  if (this.content.getBoundingClientRect().left > viewport / 2 && pulled === true) {
    this.close();
    return; // We'll never `this.open()` if this is true anyway, so combine the booleans.
  }
  this.open();
}

怎么样：

if (direction || (this.content.getBoundingClientRect().left > viewport / 2 && pulled === true)) {
  this.close();
} else {
  this.open();
}

至于：

if (this.content.getBoundingClientRect().left > viewport / 2) {
  if (this.isEmpty(delta) || delta.x > 0) {
    this.close();
    return; // Combine the booleans!
  }
  this.open();
  return;
}

简化：

if ((this.isEmpty(delta) || delta.x > 0) || !this.content.getBoundingClientRect().left > viewport / 2) {
  this.close();
} else {
  this.open();
}

（旁白：原帖遗漏了一个右括号。如果您（OP）打算让该功能在您的帖子之后继续，那么这个答案是错误的（但您应该更清楚））

结果：我们消除了两个（重复的）决定：

function () {
  var duration = +new Date() - start.time,
    isPastHalf = Number(duration) < 250 && Math.abs(delta.x) > 20 || Math.abs(delta.x) > viewport / 2,
    direction = delta.x < 0;

  if (!isScrolling) {
    if (isPastHalf) {
      if (direction || (this.content.getBoundingClientRect().left > viewport / 2 && pulled === true)) {
        this.close();
      } else {
        this.open();
      }
    } else {
      if ((this.isEmpty(delta) || delta.x > 0) || !this.content.getBoundingClientRect().left > viewport / 2) {
        this.close();
      } else {
        this.open();
      }
    }
  }
}

【讨论】：

【解决方案4】：

Bergi 已经给出了正确答案，但对我来说还是太复杂了。由于我们不是using fortran77，我认为我们最好使用early return。另外，可以通过引入额外的变量来进一步澄清代码：

function doSomething(isScrolling, start, delta, viewport) {
    if (isScrolling) return;

    var duration = +new Date() - start.time;
    var isPastHalf = Number(duration) < 250 && Math.abs(delta.x) > 20 || Math.abs(delta.x) > viewport / 2;
    var isFarRight = this.content.getBoundingClientRect().left > viewport / 2;

    // I'm not sure if my variable names reflect the actual case, but that's
    // exactly the point. By choosing the correct variable names for this,
    // anybody reading the code can immediatly comprehend what's happening.
    var isMovingToLeft = delta.x < 0;
    var isMovedPastEnd = isPastHalf && !isMovingToLeft && !(isFarRight && pulled);
    var isMovedBeforeStart = !isPastHalf && isMovingToLeft && isFarRight;

    if (isMovedPastEnd || isMovedBeforeStart) {
        this.open();
    else
        this.close();
    }
}

【讨论】：

我喜欢这个。（老实说，我没有检查细节的正确性。）但我喜欢使用足够多的布尔变量，以便对 open() 和 close() 的调用只发生一次。此外，方法顶部的返回通常有助于消除混乱。一种未显示的部分解决方案是将代码分解为多个函数。

【解决方案5】：

我更喜欢下面这样简单且嵌套较少的代码：

function() 
{
    var duration = +new Date() - start.time,
        isPastHalf = Number(duration) < 250 && Math.abs(delta.x) > 20 || Math.abs(delta.x) > viewport / 2,
        direction = delta.x < 0;

    if (isScrolling)
    {
        return;
    }
    if (isPastHalf) 
    {
        if (direction) 
        {
            this.close();
            return;
        }
        if (this.content.getBoundingClientRect().left > viewport / 2 && pulled == = true) 
        {
            this.close();
            return;
        }
        this.open();
        return;
    }
    if (this.content.getBoundingClientRect().left > viewport / 2) 
    {
        if (this.isEmpty(delta) || delta.x > 0) 
        {
            this.close();
            return;
        }
        this.open();
        return;
    }
    this.close();
    return;
}

【讨论】：