CODEWITHSUNDEEP
javascriptif-statement
cinnaroll45
cinnaroll45

Reputation: 2790

How can I refactor this bunch of if statements?

Imagine that I have a variable called incomingValue and I'm getting a number from an API as it's value. The values are between 0 to 1 and I'm setting two other variables depending on this value using bunch of if statements like you see below. 

var incomingValue; // Set by an API
var setValueName;
var setValueIcon;
if (incomingValue < 0.10 ) { 
  setValueName = 'something'; 
  setValueIcon = 'something.png' 
}
if (incomingValue > 0.09 && incomingValue < 0.20 ) { 
  setValueName = 'somethingElse'; 
  setValueIcon = 'somethingElse.png';
}

In the actual implementation, I have around 10 if statements checking for specific intervals up until 1. e.g. do this if it's more than 0.10 but less than 0.16 and so on.

As a JavaScript beginner it feels like this is not the right way to do things even though it gets the job done. How would I refactor this code?

Update: As requested, I'm adding the full set of intervals that are used in the original code. I haven't included the full list before because the intervals don't follow a certain pattern.

Upvotes: 7

Views: 433

Answers (4)

Tamas Hegedus
Tamas Hegedus

Reputation: 29906

Remember the single responsibility principle. Take that code out to a separate function like so:

function determineNameAndIcon(incomingValue) {
  if (incomingValue < 0.10) { 
    return { 
      name: "something",
      icon: "something.png"
    };
  }
  if (incomingValue < 0.20) { 
    return {
      name: "somethingElse",
      icon: "somethingElse.png"
    };
  }
  // etc
  return {
    name: "somethingDefault",
    icon: "somethingDefault.png"
  };
}

// ...

var incomingValue; // Set by an API
const {
  name: setValueName,
  icon: setValueIcon
} = determineNameAndIcon(incomingValue);

Notice that determineNameAndIcon is still a very long function with repeating parts. This can be further refactored to a data-driven version:

const nameAndIconList = [
  {
    maxValue: 0.10,
    name: "something",
    icon: "something.png"
  },
  {
    maxValue: 0.20,
    name: "somethingElse",
    icon: "somethingElse.png"
  },
  // ...
];

const nameAndIconDefault = {
  name: "somethingDefault",
  icon: "somethingDefault.png"
};

function determineNameAndIcon(incomingValue) {
  for (let item of nameAndIconList) {
    if (incomingValue < item.maxValue) {
      return item;
    }
  }
  return nameAndIconDefault;
}

Upvotes: 12

Gerardo Furtado
Gerardo Furtado

Reputation: 102174

A solution using an array where you set the ranges for your results:

var categories = [{something: [0, 0.1]}, 
                  {somethingElse: [0.1, 0.2]}, 
                  {somethingElse2: [0.2, 0.3]}, 
                  {anotherSomething: [0.3, 1]}];

function res(number){ return Object.keys(categories.filter(function(elem) {
  var key = elem[Object.keys(elem)];
  return number >= key[0] && number < key[1]
})[0])[0]};

var incomingValue = 0.12;

var setValueName = res(incomingValue);
var setValueIcon = res(incomingValue) + ".png";
console.log(setValueName, setValueIcon);

Upvotes: 1

kemiller2002
kemiller2002

Reputation: 115450

function findValue(incomingValue){

    var itemValues = [
      [.06, "valueName", "valueIcon"],  
      [.08, "valueName", "valueIcon"],
      [.09, "valueName", "valueIcon"],
      [.1, "valueName", "valueIcon"],

    ]


  var foundValues = itemValues.
        filter(v=>v[0] >= incomingValue)
        .sort();

  if(foundValues.length == 0){
     throw "incoming value not found."
  } 
  return foundValues[0];
}

let value = findValue(.079);

console.log( value );

This is assuming that you want the lowest portion of the range to be the one selected (just reverse the sort if you want it to be the highest).

Upvotes: 2

Latsuj
Latsuj

Reputation: 479

Mb I will refactor this code like this but it's not really standard patter :

var incomingValue=0.08; // Set by an API
var setValueName;
var setValueIcon;

switch(true) {
    case incomingValue < 0.10 :
        setValueName = "something"; 
        setValueIcon ="something.png";
        alert("Hello World !");
        break;
    case incomingValue > 0.09 && incomingValue < 0.20 :
        setValueName = "somethingElse"; 
        setValueIcon ="somethingElse.png";
        alert("Hello !");
        break;
    default :
        alert("Adele !");
        break;
}

The mortal will use the if...else if... condition like this :

var incomingValue; // Set by an API
var setValueName;
var setValueIcon;
if (incomingValue < 0.10 ) { 
    setValueName = "something"; 
    setValueIcon ="something.png" 
} **else** if (incomingValue > 0.09 && incomingValue < 0.20 ) { 
    setValueName = "somethingElse"; 
    setValueIcon ="somethingElse.png" 
}

But I dont like this way...My opinion :)

Upvotes: 0

Related Questions