Optimizing if statements in angular

Question

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.

"Essentially $scope.leftWordList=true & $scope.previewViewRight=truewill repeat in level 1 & 3 whereas, $scope.leftWordList=true & $scope.previewViewRight=true will repeat in level 2 & 4." Um...were those really meant to be the same? — T.J. Crowder
– T.J. Crowder, Commented Jul 17, 2016 at 12:36
I don't know Angular, but couldn't you just do $scope.counters = [] and then index them by number? — gcampbell
– gcampbell, Commented Jul 17, 2016 at 12:38
Side note: I strongly recommend being consistent with your semicolons. If you want to rely on automatic semicolon insertion, do that; if not (my recommendation), ensure you put the semicolons in where they belong. — T.J. Crowder
– T.J. Crowder, Commented Jul 17, 2016 at 12:43
Note: If all ifs and not disjoint, you should rather use if...else if...else and not if.. if — Rajesh Dixit
– Rajesh Dixit, Commented Jul 17, 2016 at 12:46
@Crowder: I updated the question, I made mistake describing the problem. Actually it looks simple now. — Thinker
– Thinker, Commented Jul 17, 2016 at 12:49

T.J. Crowder · Accepted Answer · 2016-07-17 13:44:31Z

3

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.

edited Jul 17, 2016 at 13:44

answered Jul 17, 2016 at 12:42

T.J. Crowder

1.1m201 gold badges2k silver badges2k bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Thinker Over a year ago

I had to use different counters because, when I used single counter variable, somehow counter was fast and it went in negative. It was strange.

Community · Accepted Answer · 2020-06-20 09:12:55Z

0

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.

edited Jun 20, 2020 at 9:12

CommunityBot

11 silver badge

answered Jul 17, 2016 at 13:01

Rajesh Dixit

25.1k5 gold badges52 silver badges86 bronze badges

1 Comment

Thinker Over a year ago

This solution worked, I implemented it with little modifications! Thanks!

Ziv Weissman · Accepted Answer · 2016-07-17 13:07:23Z

0

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

answered Jul 17, 2016 at 13:07

Ziv Weissman

4,5664 gold badges31 silver badges64 bronze badges

Collectives™ on Stack Overflow

Optimizing if statements in angular

3 Answers 3

1 Comment

Sample Code:

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Sample Code:

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related