Home > Software engineering >  How to refactor if blocks with repeated conditions?
How to refactor if blocks with repeated conditions?

Time:07-18

Let's say I have some 6 conditions (a-f) and if statements listed below. As you can see there is a pattern for these if statements. Every next if statement has almost the same conditions as the previous one but the first condition which was used previously is removed.

    if(a && b && c && d && e && f)
      return 5;
    if(b && c && d && e && f)
      return 4;
    if(c && d && e && f)
      return 3;
    if(d && e && f)
      return 2;
    if(e && f)
      return 1;
    if (f)
      return 0;
    return -1;

I think there is no need to have so many elaborate conditions, especially as there are cases which introduce some redundant condition checks and I would prefer to avoid that. I've tried to rewrite it, but I've only come up with something like this:

    let res = -1;
    if(f) {
      res = 0;
      if(e) {
        res = 1;
        if(d) {
          res = 2;
          if(c) {
            res = 3;
          } if(b) {
            res = 4;
            if(a) {
              res = 5;
            }
          }
        }
      }
    }
    return res;

This way I no longer have redundant condition checks, but this solution still looks overcomplicated. Are there some clever ways to rewrite it so that I can have a simple solution without redundant checks?

CodePudding user response:

Your nested if statements aren't that bad - they get rid of the duplication quite efficiently, and create a linear structure! You can further shorten the code by using a loop:

let res = -1;
for (const flag of [f, e, d, c, b, a]) {
    if (flag) res  ;
    else break;
}
return res;

This still evaluates all the conditions up-front, so if they are costly to compute you might want to use functions that you can evaluate when needed:

let res = -1;
for (const getFlag of [()=>f, ()=>e, ()=>d, ()=>c, ()=>b, ()=>a]) {
    if (getFlag()) res  ;
    else break;
}
return res;

If the conditions are cheap, and you also know that if one is false all the remaining ones are false as well, you don't even need to break the loop. It can then be simplified into a single expression:

return -1   f   e   d   c   b   a;

CodePudding user response:

Put the conditions into an array, along with the results.

const conds = [[a, 5], [b, 4], [c, 3], [d, 2], [e, 1], [f, 0]];
while (conds.length) {
  if (conds.every(([cond]) => cond)) {
    return conds[1];
  }
  conds.shift();
}
return -1;
  • Related