Refactor if statement JavaScript

Question

I was wondering on how I can refactor that? I'm repeating myself I feel this is not the best way to write it :

if (operator === "+") {
    strength += 2;
    up = 4 * strength;
    if (up > 40) up = 40;
    final.base += up;
} else if (operator === "-") {
    up = 4 * strength;
    if (up > 40) up = 40;
    final.base -= up;
    strength -= 2;
}

I don't really see a way to properly refactor that since position is important. Is there a way to clean this function?

The usual way to refactor out a large else-if ladder is to loop through an *associative array.* — Robert Harvey, Feb 08 '19 at 20:42
Have a look here: https://blog.wax-o.com/2015/05/an-alternative-to-if-else-and-switch-in-javascript/ — Robert Harvey, Feb 08 '19 at 20:43
If nothing else, the `if (up > 40) up = 40;` doesn't rely on the conditional at all. — jhpratt, Feb 08 '19 at 20:44
After some refactoring i think your original code is better. — Salman A, Feb 08 '19 at 20:45
I would say what you have, especially if there are only a few `operator`s is fine. It's readable, a reader of the code can easily identify what happens which each operator. That's likely more important. — Heretic Monkey, Feb 08 '19 at 20:46
@jhpratt I don't really understand your comment. up is kind of a limit. You can't add/substract more than 40 even if the result of 4*strength is over 40. — Zeyukan Ich', Feb 08 '19 at 20:48
https://stackoverflow.com/questions/5834318/are-variable-operators-possible — Teemu, Feb 08 '19 at 20:50

Nina Scholz · Accepted Answer · 2019-02-08T21:24:52.663

9

You could write it more compact, if you do not use up later by using Math.min.

if (operator === "+") {
    strength += 2;
    final.base += Math.min(40, 4 * strength);
} else if (operator === "-") {
    final.base -= Math.min(40, 4 * strength);
    strength -= 2;
}

edited Feb 08 '19 at 21:24

answered Feb 08 '19 at 20:46

Nina Scholz

376,160
25
347
392

This is an elegant idea and it respects position, thanks! – Zeyukan Ich' Feb 08 '19 at 20:53
1

@FlickaMin if you want to update `up` and increment `base` at the same time, `final.base += up = Math.min(40, 4 * strength)` – adiga Feb 08 '19 at 20:56
The second approach won't work, note that `final.base` is not always incremented... – Shidersz Feb 08 '19 at 21:11
@Shidersz, that's why i added a disclaimer. – Nina Scholz Feb 08 '19 at 21:14
I think,you do not understand me. I believe you forgot to replace `final.base += Math.min(40, 4 * strength);` by this `final.base = final.base + (operator === "+" ? Math.min(40, 4 * strength) : -Math.min(40, 4 * strength));` or maybe by this: `final.base += (operator === "+" ? 1 : -1) * Math.min(40, 4 * strength);` – Shidersz Feb 08 '19 at 21:21
@Shidersz, right. i missed the different operator. thank you! – Nina Scholz Feb 08 '19 at 21:25

Shidersz · Answer 2 · 2019-02-08T21:39:02.270

I will do something like this, for preserve up variable:

if (operator === "+")
{
    up = Math.min(4 * (strength += 2), 40);
    final.base += up;
}
else if (operator === "-")
{
    final.base -= (up = Math.min(4 * strength, 40));
    strength -= 2;
}

If you don't need up variable, can be simplified to this:

if (operator === "+")
{
    final.base += Math.min(4 * (strength += 2), 40);
}
else if (operator === "-")
{
    final.base -= Math.min(4 * strength, 40);
    strength -= 2;
}

If you don't need up variable and also you only have + and - operators, then you can go like this:

strength += (operator === "+") ? 2 : 0;
final.base += (operator === "+" ? 1 : -1) * Math.min(4 * strength, 40);
strength -= (operator === "-") ? 2 : 0;

num8er · Answer 3 · 2019-02-08T21:34:49.150

My answer is not refactoring of if..else - it's about thinking ahead how Your app can grow, it's about making right choices.

In large apps with complex logic You'll have to abstract methods to make You code more flexible.

For example how about having Operations class that abstracts if..else switch, which You can extend?

class Operations {
  static plus (base, strength) {
    base = parseInt(base);
    strength = parseInt(strength);
    
    strength += 2;
    base += Math.min(40, 4 * strength);
    
    return [base, strength];
  }
  
  static minus (base, strength) {
    base = parseInt(base);
    strength = parseInt(strength);
    
    base -= Math.min(40, 4 * strength);
    strength -= 2;
    
    return [base, strength];
  }

  static do (operation) {
    const operators = {
      '+' : Operations.plus,
      '-' : Operations.minus
    }

    const args = Object.values(arguments).slice(1);
    
    if (!operators[operation]) {
      return args;
    }

    return operators[operation].apply(null, args);
  }
}

   
let final = {base: 10};
let strength = 10;
let newBase, newStrength;

console.log('Before. base:', final.base, 'strength:', strength);

// NO IF ELSE ON OPERATOR (:
[newBase, newStrength] = Operations.do('+', final.base, strength);
strength = newStrength;
final.base = newBase;

console.log('After "+" operation. base:', final.base, 'strength:', strength);

[newBase, newStrength] = Operations.do('-', final.base, strength);
strength = newStrength;
final.base = newBase;

console.log('After "-" operation. base:', final.base, 'strength:', strength);

molamk · Answer 4 · 2019-02-08T20:54:08.837

1

To solve the duplication problem, you can add a multiplier factor since the majority of what's changing here is just the sign.

let multiplier = 1;
if (operator === "-")
    multiplier = -1;

up = 4 * strength;
strength += multiplier * 2;
if (up > 40) up = 40;
final.base += multiplier * up;

Note

This will only work if operator is either - or +. If it's something like *, this will act as if the operator is +

edited Feb 08 '19 at 20:54

answered Feb 08 '19 at 20:46

molamk

4,076
1
13
22

As I said just before, position is really important so I can't do something like this. If the operator is "plus" I must get the up after incrementing, if the operator is "minus" it's after. – Zeyukan Ich' Feb 08 '19 at 20:49
yeah you're right @num8er. This will only work if the operator is either `+` or `-`. Good catch, I'm adding an edit – molamk Feb 08 '19 at 20:53

Taki · Answer 5 · 2019-02-08T21:01:29.690

you could put the operations in an object :

const obj = {
    "+" : {
        strength : prevStrength => prevStrength + 2,
        finalBase: (prevFinalBase , up) =>  prevFinalBase + Math.min(40, 4 * strength)
    },  
    "-" : {
        strength : prevStrength => prevStrength - 2,
        finalBase: (prevFinalBase , up) => prevFinalBase - Math.min(40, 4 * strength)
    }
}

strength = obj[operator].strength(strength);
finalBase = obj[operator].finalBase(finalBase);

var operator = "+";
var strength = 3;
var finalBase = 5;

const obj = {
  "+": {
    strength: prevStrength => prevStrength + 2,
    finalBase: (prevFinalBase, up) => prevFinalBase + Math.min(40, 4 * strength)
  },
  "-": {
    strength: prevStrength => prevStrength - 2,
    finalBase: (prevFinalBase, up) => prevFinalBase - Math.min(40, 4 * strength)
  }
}

strength = obj[operator].strength(strength);
finalBase = obj[operator].finalBase(finalBase);

console.log({
  strength,
  finalBase
})

score 0 · Answer 6 · edited Feb 08 '19 at 20:49

0

You could consider writing the logic as a series of conditional ternary operator statements:

let isAdd = operator === '+'
strength += isAdd ? 2 : 0;
up = 4 * strength;
if(up > 40) up = 40;
final.base += isAdd ? up : (-1 * up)
strength -= isAdd ? 0 : 2;

edited Feb 08 '19 at 20:49

Jack Bashford

43,180
11
50
79

answered Feb 08 '19 at 20:48

Braden W

91
1
4

True! I think there are much better answers above! – Braden W Feb 08 '19 at 23:34

score 0 · Answer 7 · answered Feb 08 '19 at 21:03

0

How about this ?

 /*ASCII Code For "-" = 45, "+" = 43*/
 operator = (44-operator.charCodeAt(0));
 if(operator > 0) strength += 2;
 up = 4*strength;
 if(up > 40) up = 40;
 final.base = final.base + (operator*up);
 if(operator < 0) strength -= 2;

answered Feb 08 '19 at 21:03

genestiel

104
5

Refactor if statement JavaScript

7 Answers7

Note