I have a horrible nested if. There could in the future be even more lines.

if (people < 10) {
    price = 500;
} else if (people >= 10 && people < 25) {
    price = 350;
} else if (people >= 25 && people < 100) {
    price = 250;
} else if (people >= 100) {
    price = 200;
}

The price goes down as the volume goes up. How do I refactor this to make it more maintainable/readable?

Edit: I tried a switch and it was not any better?

I have a horrible nested if. There could in the future be even more lines.

if (people < 10) {
    price = 500;
} else if (people >= 10 && people < 25) {
    price = 350;
} else if (people >= 25 && people < 100) {
    price = 250;
} else if (people >= 100) {
    price = 200;
}

The price goes down as the volume goes up. How do I refactor this to make it more maintainable/readable?

Edit: I tried a switch and it was not any better?

• javascript
• if-statement
• refactoring

Share Improve this question Follow edited Nov 8, 2018 at 10:07 TommyD asked Nov 8, 2018 at 10:02 TommyDTommyD 1,04355 gold badges1919 silver badges4141 bronze badges 3

• Yes sorry a typo. Fixed. – TommyD Commented Nov 8, 2018 at 10:07
• I tried a switch.. And why didn't you like it? – hindmost Commented Nov 8, 2018 at 10:09
• 1 Passing (true) into it seemed a bit hacky, plus there was little difference to the nested if. – TommyD Commented Nov 8, 2018 at 19:07

Add a ment  | 

5 Answers 5

Sorted by: Reset to default 4

One option would be to use an array that defines the thresholds, then .find the appropriate value in the array. This will be very concise, especially when there are lots of thresholds:

const thresholds = [
  [100, 200], // need 100+ people for the price to be 200
  [25, 250], // else need 25+ people for the price to be 250
  [10, 350],
  [0, 500]
];
function findPrice(people) {
  return thresholds.find(([limit]) => people >= limit)[1];
}

console.log(findPrice(53)); // 53 people
console.log(findPrice(25));
console.log(findPrice(24));

You could take a function with early exit. The previous check is the condition for the next check or for getting the maximum result.

The advantage is to prevent chains of else ifstatements and to offer a better maintanability.

function getPrice(people) {
    if (people < 10) {
        return 500;
    } 
    if (people < 25) {
        return 350;
    }
    if (people < 100) {
        return 250;
    }
    return 200;
}

var price = getPrice(people);

More to read:

• Should I return from a function early or use an if statement?

Well you dont need the check for >=, when the check will stay in this form:

if (people < 10) {
    price = 500; 
} else if (people < 25) { 
    price = 350;
} else if (people < 100) { 
    price = 250; 
} else { 
    //people count is implicitly greater than 100
    price = 200; 
}

On each (next) step the people count is implicitly greater than the previous check, so eg. if people < 10 results in false the value is implicitly greater than 9 or >= 10. For this reason the duplicate check is not needed an thus can be omitted.

function applyConf(v) {
  return [{
    // false means infinite
    min: false,
    max: 9,
    value: 500,
  }, {
    min: 10,
    max: 24,
    value: 350,
  }, {
    min: 25,
    max: 99,
    value: 250,
  }, {
    min: 100,
    max: false,
    value: 200,
  }].find(({
    min,
    max,
  }) => (min === false || v >= min) && (max === false || v <= max)).value;
}

console.log(applyConf(-10));
console.log(applyConf(8));
console.log(applyConf(20));
console.log(applyConf(80));
console.log(applyConf(100));
console.log(applyConf(100000));

I would prefer instead of to many if to use switch condition as below

function getPrice(people)
{
    switch(true){
        case people<10: return 500;
        case people<25: return 350;
        case people<100: return 250;
        default: return 200;
    }

}