0

I am trying to refactor if..else statements.

My code:

let condition = 'hi';

if(condition === 'hi'){
  commonFunction('hi');
  console.log('hi is called');

}else if(condition === 'bye'){
  commonFunction('bye');
  console.log('bye is called');

}else if(condition.includes('happy')){
  commonFunction('happy');
  console.log('happy is called');

}else if(condition === 'greeting'){
  commonFunction('greeting');
  console.log('greeting is called');

}

Refactored Code:

if(condition === 'hi'){
  hi();
}else if(condition === 'bye'){
  bye();
}else if(condition.includes('happy')){
  happy();
}else if(condition === 'greeting'){
  greeting();
}

function hi(){
  commonFunction('hi');
  console.log('hi is called');
}

function bye(){
  commonFunction('bye');
  console.log('bye is called');
}

function happy(){
  commonFunction('happy');
  console.log('happy is called');
}

function greeting(){
  commonFunction('greeting');
  console.log('greeting is called');
}

Is it better to declare each functions by condition like my refactored code???

Or, How about make class and call commonFunction by constructor? (I think switch..case is not useful becasue I have a condition that has includes() )

A J
  • 1,439
  • 4
  • 25
  • 42
HanJohn
  • 1
  • 1
  • 1
  • @lucumt I have includes() in my condition so I think I can't use switch – HanJohn Nov 27 '18 at 02:48
  • this is really relative to how dynamic your functions will be. In your example, you could just declare 1 function and pass 2 parameters. Also you can call function by string: https://stackoverflow.com/a/1144334/10412708 – ACD Nov 27 '18 at 02:50
  • In the above case, can't we do something like below? const functionToCall = condtion.includes('hello') ? 'hello' : condtion; component(functionToCall); – Goutham May 04 '21 at 07:35

6 Answers6

1

One option would be to use an object containing functions, rather than having multiple standalone functions. Then, just use property lookup.

For this exact code, though, you could make it even DRY-er by using a Proxy to check which property was accessed, rather than having multiple properties (because all of your properties/functions have common functionality):

const condition = 'hi';

const fns = new Proxy({}, { get: (_, prop) => {
  console.log(prop + ' is called');
}});

const props = ['hi', 'bye', 'greeting'];
if (condition.includes('happy')) {
  fns.happy();
} else if (fns[condition]) {
  fns[condition]();
}

This is extremely DRY, but pretty odd to do - in most situations, you'd use an object instead:

const fns = {
  hi() {
    console.log('hi is called');
  },
  bye() {
    console.log('bye is called');
  },
  greeting() {
    console.log('greeting is called');
  },
  happy() {
    console.log('happy is called');
  }
}

const condition = 'hi';

if (condition in fns) {
  fns[condition]();
} else if (condition.includes('happy')) {
  fns.happy();
}
CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
1

I'd suggest to move all these if-elses into commonFunction itself (if it does not contain the same if-elses again, if it does you need more complex refactoring):

const commonFunction = (condition) => {
  let f = ['hi', 'bye', 'happy', 'greeting'].find(e => condition.includes(e))
  console.log(f + ' is called');
  if (!f) return;
  // do something
  // return the result if needed
}

commonFunction('greeting')
commonFunction('die')
Kosh
  • 16,966
  • 2
  • 19
  • 34
0

One thing can do about that is declare the function in an object using the condition as keys.

let objFunc = {
    'hi': function() {
       commonFunction('hi');
       console.log('hi is called');
    },
    'happy': function() {
       commonFunction('happy');
       console.log('happy is called');
    },
    'bye': function() {
       commonFunction('bye');
       console.log('bye is called');
    },
    'greeting': function() {
       commonFunction('greeting');
       console.log('greeting is called');
    },
};


objFunc[condition]();
onecompileman
  • 900
  • 7
  • 14
  • However, I have condition that using includes()... How can I using the condition as keys in this situation? – HanJohn Nov 27 '18 at 02:52
0

In your original code you have written multiple if else statement just to call same function with different pre-defined argument, so instead of refactoring by your way, i think this is much easier and small code.

let predefinedArgs = {
    hi: 'hi',
    bye: 'bye'
    ...
}

let condition = 'hi'
commonFunction(predefinedArgs[`condition`]);
Atul Bansode
  • 211
  • 1
  • 5
0

You have a set of conditions for which you want to call a common function. The best way to do this is simply test whether you can handle the condition (that's where you can use includes). Then call that function. 

const allowed = ['hi', 'bye', 'happy', 'greeting']

let condition = 'happy'

if (allowed.includes(condition)) {           // do you know how to deal with condition? 
  commonFunction(condition)                 // if so call the function
} else {
  console.log(`unknown condition ${condition}`) // otherwise log an error
}

// not sure what commonFunction does, so this is just for the example
function commonFunction(condition) {
  console.log(`${condition} is called`);
}
Mark
  • 90,562
  • 7
  • 108
  • 148
0

I think all the solutions should work fine. In respect of the condition which has includes, maybe you can try to add in a new parameter. What I meant was, as stated in earlier answers:

  let f = ['hi', 'bye', 'happy', 'greeting'].find(e => condition.includes(e))
  console.log(f + ' is called');
  if (!f) return;
  // do something
  // return the result if needed
}

commonFunction('greeting')
commonFunction('die')```

This could be converted to :
```const commonFunction = (condition, functionName) => {
// ... other code
// and then use can use the following link to call the function which you want to execute`
https://stackoverflow.com/questions/53492013/javascript-refactoring-if-else-statements-es6/53492079
const young = () => {//...}
and the calling could be:
commonFunction('greeting', slice)
commonFunction('die', young)```

Hope this makes sense!
Prashant
  • 31
  • 2