javascript 我怎样才能减少圈复杂度？

how could I reduce the cyclomatic complexity?

提问by Roland

Whenever I lint a piece of code I'm working on I get the This function's cyclomatic complexity is too high. (7). But I'm a bit confused on how I could rewrite it in such way so it works.

每当我整理一段我正在处理的代码时，我都会得到This function's cyclomatic complexity is too high. (7). 但是我对如何以这种方式重写它以便它起作用感到有些困惑。

This would be the function that keeps throwing that message:

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

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

I would like to hear some advice on how I could structure my code in such way so I avoid this kind of situations.

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

回答by Bergi

Well you have only two actions in your code, but much too many conditions. Use a single if-else-statement, and boolean operators in the condition. If that was impossible, you could at least

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

• remove the empty lines to get the full logic on one screen page
• add some comments on what the branches are doing (and why)
• avoid early returns

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

Here's your function simplified:

这是您的功能简化：

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

and shortened:

并缩短：

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

回答by David Miani

Firstly, there are three results your function can have: do nothing, call this.close()or call this.open(). So ideally the resulting function will just have one if statement which determines which result is used.

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

The next step is to extract all boolean code into variables. Eg var leftPastCenter = this.content.getBoundingClientRect().left > viewport / 2.

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

Finally, use boolean logic to simplify it step by step.

最后，使用布尔逻辑一步一步地简化它。

Here is how I did it:

这是我如何做到的：

Firstly, extract all boolean variables:

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

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

The easiest part to pull out is realizing if isScrollingis true, nothing ever happens. This immediately gets rid of one level of nesting:

最容易退出的部分是意识到如果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();
    }
}

Now look at the cases this.open()are called. If isPastHalfis true, this.open()is only called when !directionand !(leftPastCenter && isPulled). If isPastHalfis false, then this.open()is only called when leftPastCenterand !positiveDelta:

现在来看一下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();
        }
    }

Flipping the ifs (so this.close()comes first), makes the code a bit neater, and gives my final version:

翻转 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();
        }
    }
}

It is difficult for me to do more, without knowing your codebase. One thing to note is directionand my new variable positiveDeltaare nearly identical - you could possible remove positiveDeltaand just use direction. Also, directionisn't a good name for a boolean, something like movingLeftwould be better.

在不了解您的代码库的情况下，我很难做更多的事情。需要注意的一件事是direction，我的新变量positiveDelta几乎相同 - 您可以删除positiveDelta并仅使用direction. 此外，direction对于布尔值来说，这不是一个好名字，像movingLeft这样的名字会更好。

回答by kojiro

Actually all those returnstatements are confusing the issue, but they offer a hint to the solution.

实际上，所有这些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();
}

How about:

怎么样：

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

And as for:

至于：

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

Simplify:

简化：

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

(Aside: The original post left out a closing brace. If you (OP) intended that the function continues past your post, then this answer is wrong (but you should've made that clearer))

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

Result: We've eliminated two (repeated) decisions:

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

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

回答by realbart

Bergi has already given a correct answer, but it's still too complex for my taste. Since we're not using fortran77I think we're better off using an early return. Also, the code may be further clarified by introducing extra variables:

Bergi 已经给出了正确的答案，但我的口味还是太复杂了。由于我们没有使用 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();
    }
} 

回答by TuneFanta

I would prefer a simple and less nested code like below:

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

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