Which would you prefer and why?

I recently wrote a small function. And thought of different ways of implementing it.

Basically the function accepts a parameter and do string manipulation.
Which would be preferred?

function (arg) {
  let optionResult = ''

  if (arg === 'option1') {
    optionResult = // do stuff for option1
  } else if (arg === 'option2') {
    optionResult = // do stuff for option2
  } else if (arg === 'option3') {
    optionResult = // do stuff for option3
  }

  return optionResult
}

or instead

function (arg) {
  let optionResult = {
    'option1': // do stuff for option1,
    'option2': // do stuff for option2,
    'option3': // do stuff for option3
  }

  return optionResult[arg]
}

Comments

[Adrian B.G.]

Either way I'll not use if for more than 2 options, and because the devs are usually lazy and do not refactor and just add a new option, I tend to never use it.

The answer depends a lot on the size of the do stuff code blocks.

1. 1-2 liners I suggest using a switch

2. If there are more I would suggest using a map, similar to your 2nd version but it can be replaced with other functions. This is how actually some compilers transform the switch.

const handlers = {};
handlers[`option1`] = function(){ //do a lot of stuff }
handlers[`option2`] = function(){ //do a lot of stuff }
function (arg) {
  //if arg doesnt exists in handlers ..
  return handlers[arg]();
}

Because of the size of the function (now in the x * 10s) I would transform this function to a module, and declare the option functions outside of the main function.

The code can grow independently, the functions will not be "compiled" until they are called for the first time and they can be tested independently, if needed.

3 . 2 can be future extended, if needed to one of Plugin/Services/Inversion of control pattern, mainly to inject new functionalities in the map as more modules are active in your project.

[Sergiu Mureşan]

This is a great improvement over the second version.

The only issue I find is that the code for the function is no longer inside the function itself but outside. Maybe this will solve the issues:

let handlers;
function (arg) {
  if (!handlers) {
    handlers[`option1`] = function() { //do a lot of stuff }
    handlers[`option2`] = function() { //do a lot of stuff }
  }
  return handlers[arg];
}

[Adrian B.G.]

Functions in functions I would say is smelly code, anyway if there are 5 options each with 7+ LOC then the main function will become too big, that is why I said to make a distinct object/module, with a size of 50+ LOC would probably required its own module. Then the functions being outside is ok, because they are encapsulated by the module.

[Sergey Kislyakov]

The second example looks a lot nicer to me.

Note 1: you can use const instead of let in your second example because you do not transform the object anywhere in your function.

Note 2: second example will return undefined instead of '' if the key in the object does not exist. I think you'd handle that as well.

[Shalvah]

*because you do not reassign the reference.

You can transform an object variable declared with const.

[Omar Lozada]

Thanks for pointing out the difference between let and const, I'll keep that in mind.

[Sergiu Mureşan]

The first option is best here for 3 reasons:

• It has better performance (only processes the option it lands on, not all of them)
• More intuitive in the long run (if the function becomes much larger then it is hard to understand that the object optionResult is actually a switch or if else
• You are returning an empty string if arg has an unexpected value, whereas in the second version you can't do that unless you check if optionResult doesn't have the property which further adds to the complexity of the function.

You can also straight up return the optionResult inside the if statements if no post-processing is needed.

[Omar Lozada]

Thanks for the detailed response,
I agree that in the code above I haven't considered doing some validation.

[Brett Stevenson]

I agree with the other responses that the second function looks much cleaner in this example, though it could also become less intuitive as the complexity of the logic that replaces // do stuff for option(n) increases.

[simo]

Ternary Operator:

const foo = (arg) =>
  arg === 'option1'
  ? 'foo' // do stuff for option1

  : arg === 'option2'
  ? 'bar' // do stuff for option2

  : arg === 'option3'
  ? 'baz' // do stuff for option3

  : 'noop'

[misbeliever]

That looks ugly

[simo]

I had to say why do I prefer it and why, right?

1. Well, that's the most elegant way to do it, without creating an object just to return the value in the end.
2. It's better than using switch, no fall-through footgun here.
3. It's obviously more elegant than if-else-return.
4. Implicitly returns a value - essential for functional programming (that can be achieved with the object approach too).

While it might not be the prettiest code to see - it's what we currently have in the language, also it's pretty straightforward to understand.

[dallgoot]

Let's split on the principles that describe the needs:

• function checks 'arg' value among known values (implies constants)
• function then process a treatment according to that value
• if the value is unknown/invalid : fallback or error

function onOption(arg) {
    switch (arg) {
        case OPTION1 : return onOption1();
        case OPTION2 : return onOption2();
        case OPTION3 : return onOption3();
        default:
            return onUnknownOption()
    }
}

Results:

• the accepted values are used through constants: consistency through code and a single place for modifications if need to be.
• The function do a simple thing : branching against 'arg' value according to a set of known values (unless invalid).
• Every specific process is encapsulated in a specific function: if a treatment is shared between option : another function can encapsulate it and make it reusable, where if it were inside the switch it would mean copy/pasting (or worse trying to factorize using IF). If "onOptionX" methods are pure, this is a pure function also.

I see often people trying to replace a switch with an object.
On the eye it's pleasant but in reality it's not using the simplest language construct to do the job, it's allocating memory for an object only to perform a string comparison.
Moreover many people don't take in account the "key not found" scenario leading to functions that errors or return undefined.

[Richard Kichenama]

Using an object, where the keys are unique and translate to an action or value, is preferred because it reduces the complexity to a single path instead of multiple with switch or if. Can be externalized into a constant and have the function be

const Mutations = {
  'option1': ...,
};

function (arg) {
  const result = Mutations[arg];
  return (result && result) || `unknown option: ${arg}`;
}

[Michael Salaverry]

If only one of the string manipulation functions should be run, then I would recommend ramdajs.com/docs/#cond which will only run the first condition which matches. The switch statement could run multiple times if a break or return is forgotten.

[misbeliever]

2 option is much better, but the function still not the best in the best form , optionalResult is created everytime function is used, if its used multiple times, that is a waste of time, so it should be initialized out of scope, and best passed as an argument to the function to avoid sideefects.

[Chris James]

Why not just switch?

developer.mozilla.org/en-US/docs/W...

[Omar Lozada]

Yeah, been thinking of using switch statement.

It's just that for me it looked much harder to read at first compared to using if statement.