how could I reduce the cyclomatic complexity?

Roland picture Roland · Jul 29, 2013 · Viewed 32.8k times · Source

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 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.

Answer

Bergi picture Bergi · Jul 29, 2013

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

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