CODEWITHSUNDEEP
javascriptangularjs
Thinker
Thinker

Reputation: 5356

Optimizing if statements in angular

I have following code with multiple 'if' statements. 

if($scope.level===1){

    $scope.leftWordList=true
    $scope.previewViewRight=true

    $scope.counter1=5
    $timeout($scope.startFilling, 5000)
    $scope.onTimeout = function(){

      $scope.counter1--;
      mytimeout = $timeout($scope.onTimeout,1000);

      if($scope.counter1==0){
        $timeout.cancel(mytimeout);
        $scope.counter=0
      }
    }
    var mytimeout = $timeout($scope.onTimeout,1000); 

  }

  if($scope.level===2){

    console.log("Level 2")

    $scope.leftWordList=true
    $scope.previewViewRight=true

    $scope.counter2=5
    $timeout($scope.startFilling, 5000)
    $scope.onTimeout = function(){

      $scope.counter2--;
      mytimeout = $timeout($scope.onTimeout,1000);

      if($scope.counter2==0){
        $timeout.cancel(mytimeout);
      }
    }
    var mytimeout = $timeout($scope.onTimeout,1000); 
  }
  ....

$scope.level goes on till 7, and most of the code inside 'if' is same except for few statements, so I guess there is definitely a scope for optimizing it, but I do not exactly know how.

How can I do this?

UPDATE: Removed incorrect description of problem statement.

Upvotes: 1

Views: 91

Answers (3)

T.J. Crowder
T.J. Crowder

Reputation: 1074495

When you find yourself with a long list of mutually-exclusive branches like that, the question becomes: Are the branches fundamentally different, or are there commonalities that can be factored?

If they're fundamentally different, either a switch or a function dispatch table immediately comes to mind.

But in your case, it looks a lot like they're just the same logic with a different counter. If so, remove your individual counter properties (counter1, counter2, etc.) and replace them with an array of counters you can index into.

Then grab the level (because if the timeout, you want a consistent value rather than dealing with it having changed before the timeout occurred) and use that throughout, see the *** lines:

var level = $scope.level;                      // ***
console.log("Level " + level)                  // ***

$scope.leftWordList = true
$scope.previewViewRight = true

$scope.counters[level] = 5                     // ***
$timeout($scope.startFilling, 5000)
$scope.onTimeout = function() {

    $scope.counters[level]--;                  // ***
    mytimeout = $timeout($scope.onTimeout, 1000);

    if ($scope.counter[level] == 0) {          // ***
        $timeout.cancel(mytimeout);
    }
}
var mytimeout = $timeout($scope.onTimeout, 1000);

Note that this assumes all of this code is within a function and so the mytimeout and level aren't shared with other invocations of the function.

Upvotes: 3

Ziv Weissman
Ziv Weissman

Reputation: 4526

You can build an object that will help you.

The object will contain all your defenitions of the things that differ from level to level, and the rest you can place inside a "switch-case"

Something like this:

lvlObjects = [];
lvlObject = {};
lvlObject.level = 1;
lvlObject.leftWordList = true;
lvlObject.previewViewRight = true;
....
lvlObjects.push(lvlObject);
//same for more levels.
//then call to a function that fill the scope with current values:
function updateLevel(currentLevel) {
    $scope.level = lvlObjects[currentLevel-1].level; //if needed
    $scope.leftWordList = lvlObjects[currentLevel-1].leftWordList;
    ....
}

Upvotes: 0

Rajesh
Rajesh

Reputation: 24925

Based on given code, you can make it generic if you change a bit:

  • You can have a counter object and then based on level, you can update necessary flag.
  • If you have any common logic that is applicable for most, then add it before your ifs and reset then in necessary if.
  • If your counter has reached 0, you should not initiate a timeout and then clear it. Just check condition before initializing it.
  • Your function onTimeout has a similar signature in both ifs. Try to make it a function, rather than copying it in all ifs

Sample Code:

$scope.counter = {
  counter1: null,
  counter2: null,
}

.
.
.

// if block
$scope.leftWordList = true;
$scope.previewViewRight = true;

$scope.counter["counter" + $scope.level] = 5
$timeout($scope.startFilling, 5000)
var mytimeout = $timeout($scope.onTimeout.bind(this, $scope.counter, $scope.level), 1000);

.
.
.

// Generic function
$scope.onTimeout = function(counter, level) {
  if (--$scope.counter["counter" + level] === 0) 
    mytimeout = $timeout($scope.onTimeout, 1000);
  else
    $scope.counter = 0    
}

Note: I'm assuming you are setting $scope.counter somewhere else. Hence I have reset it in else condition.

Upvotes: 0

Related Questions