CODEWITHSUNDEEP
javascriptrefactoringconditional-statementscyclomatic-complexity
antonpug
antonpug

Reputation: 14286

How to refactor a long else if statement?

My code piece reached a cyclomatic limit, trying to think of a way to refactor. 

if(item.oldLabelType === 'Fruits') {
  item.newLabel = this._processFruitsLabel(item.oldLabel);
}
else if(item.oldLabelType === 'Vegetables') {
  item.newLabel = this._processVegetablesLabel(item.oldLabel);
}
else if(item.oldLabelType === 'Animals') {
  item.newLabel = this._processAnimalsLabel(item.oldLabel);
}
else if(item.oldLabelType === 'Fish') {
  item.newLabel = this._processFishLabel(item.oldLabel);
}
else if(item.oldLabelType === 'Birds') {
  item.newLabel = this._processBirdsLabel(item.oldLabel);
}
else if(item.oldLabelType === 'Colors') {
  item.newLabel = this._processColorsLabel(item.oldLabel);
}
else if(item.oldLabelType === 'Countries') {
  item.newLabel = this._processCountriesLabel(item.oldLabel);
}
else if(item.oldLabelType === 'Drinks') {
  item.newLabel = this._processDrinksLabel(item.oldLabel);
}
else if(item.oldLabelType === 'Cars' || item.oldLabelType === 'Airplanes') {
  item.newLabel = this._processTransportationLabel(item.oldLabel);
}

Synopsis - I'm in the process of refactoring a code base, the back end returns undesirable values, i.e. old label for something might be "Only $1000", the new label needs to "You pay only $1000 today.". The label manipulation is radically different depending on the item.oldLabelType that's sent back. So I can't really write a one-size-fits-all function that will transform any and all of the old labels into new.

What to do!?

Upvotes: 3

Views: 2872

Answers (4)

maxdeviant
maxdeviant

Reputation: 1179

Since functions are first-class citizens in JavaScript, we can do something like this:

var labelMapping = {
  Fruits: this._processFruitsLabel,
  Vegetables: this._processVegetablesLabel,
  Animals: this._processAnimalsLabel,
  Fish: this._processFishLabel,
  Birds: this._processBirdsLabel,
  Colors: this._processColorsLabel,
  Countries: this._processCountriesLabel,
  Drinks: this._processDrinksLabel,
  Cars: this._processTransportationLabel,
  Airplanes: this._processTransportationLabel
};

var processFn = labelMapping[item.oldLabelType];

if (typeof processFn === 'function') {
  item.newLabel = processFn(item.oldLabel);
} else {
  // Handle when we can't find the process function.
}

One caveat is that if you use this inside of the various process functions, then you'll need to make sure they get called with the right this context.

There are two ways of doing this:

  1. .bind the functions ahead of time

Fruits: this._processFruitsLabel.bind(this),

  1. Use .call and pass in the current this

item.newLabel = processFn.call(this, item.oldLabel);

Upvotes: 2

zfrisch
zfrisch

Reputation: 8660

If most of your functions being called are simply reiterations of each other with a slight variation in name based on the label type, you can create the correct function name to call on the fly. In the event that any function name houses many label types you can use a switch statement to specify the groups function name.

if (item.oldLabelType) {
   let olt = item.oldLabelType;
   switch (olt) {
     case "Airplane":
     case "Car":
       olt = "Traffic"
       break;
   }
   let func = "_process" + olt + "Label";
   item.newLabel = this[func](item.oldLabel);
 }

Upvotes: 0

Koushik Chatterjee
Koushik Chatterjee

Reputation: 4175

construc a object for map to the correct function at begining like this:

var targetFunctionMap = {
    "Fruits": this._processFruitsLabel,
    "Vegetables": this._processVegetablesLabel,
    "Animals": this._processAnimalsLabel
    .............
    .............
    .............
    "Cars": this._processTransportationLabel,
    "Airplanes": this._processTransportationLabel
}

and then invoke from there 

item.newLabel = targetFunctionMap[item.oldLabelType].call(this);

Upvotes: 1

T.J. Crowder
T.J. Crowder

Reputation: 1074465

The usual answers here are:

  1. Use a switch (but it's not much of an improvement, or arguably an improvement at all)
  2. Use a lookup Map or object
  3. Use string concatenation and brackets notation

Here's how that third one would look:

var functionName = item.oldLabelType === 'Cars' || item.oldLabelType === 'Airplanes'
    ? "_processTransportationLabel"
    : "_process" + item.oldLabelType + "Label";
if (this[functionName]) {
    item.newLabel = this[functionName](item.oldLabel);
}

Upvotes: 4

Related Questions