Liz.
Liz.

Reputation: 815

reduce complexity of if elseif conditions

I have one function which is having if elseif conditions and the cyclomatic complexity is approaching 5. How do I reduce it?

function testFunc() {
    var step = getModel('step');
    if(step === 1) {
        this.resetTask(); //calling some function
        this.updateStep(0);
        return true;
    } else if(step === 2) {
        this.initTask; //some other function
        return true;
    } else if(step === 3) {
        this.name === 'add' ? this.add() : this.edit();
        return true;
    }
    return false;
}

tried replacing with switch case but it didn't help.

Upvotes: 1

Views: 2304

Answers (3)

VLAZ
VLAZ

Reputation: 28994

Very simple refactor - remove all conditional logic you have now and extract each piece as a separate function into a map. Since you only execute one branch each time, depending on step, you can make that value the key and fetch what needs to be executed. You can then provide a fallback for when there is nothing that corresponds to step.

Now the cyclomatic complexity is 2, since there is only one place the code branches - either you find the corresponding handler for step or not. Also, the branching in step 3 is a completely separate function now, thus it doesn't need to count as part of testFunc

function testFunc() {
  var step = getModel('step');

  var steps = {
    1: function() {
      this.editTask(); //calling some function
      this.updateStep(0);
      return true;
    },
    2: function() {
      this.initTask; //some other function
      return true;
    },
    3: function() {
      this.name === 'add' ? this.add() : this.edit();
      return true;
    },
    default: function() { 
      return false; 
    }
  };

  var fn = steps[step] || steps.default;

  return fn();
}

Upvotes: 3

Shubham
Shubham

Reputation: 779

This will reduce your code complexity and makes more readable. You can decouple your condition into multiple function and attach that function in condition object. here I'm passing this in argument but you can use different approch. here I've also given value of this just for the shake of running example. copy and paste in your editor and try.

function meaningfulFuncName1() {
  this.editTask(); //calling some function
  this.updateStep(0);
  return true;
}

function meaningfulFuncName2() {
  this.initTask; //some other function
  return true;
}

function meaningfulFuncName3(context) {
  context.name === 'add' ? context.add() : context.edit();
  return true;
}

function defaultCase() {
  return false;
}

function testFunc() {
  this.name = 'add'
  const step = getModel('step')
  conditionObject = {
    1: meaningfulFuncName1,
    2: meaningfulFuncName2,
    3: meaningfulFuncName3,
    default: defaultCase
  }
  return conditionObject[step](this);
}

Upvotes: 3

Jan
Jan

Reputation: 21

To me, changing it like this makes it less complex to understand. Although to some will be less readable. Also it is a little faster.

function testFunc() {
 var step = getModel('step');
 if(step==1){this.editTask(); this.updateStep(0); return true;} 
 if(step==2){this.initTask; return true;} 
 if(step==3){this.name==='add' ? this.add() : this.edit();return true;}
 return false;
}

Upvotes: -2

Related Questions