if (!_if) what

From time to time, the use of if statements causes a bit of debate in my computing circles (it's funny to hear us start arguments with "if you use if..."). Most recently, I came across this post. In one of the comments, an assertion was made that if statements should be avoided since they represent design flaws. While I don't agree that the existence of if statements in code are all bad, I was inspired to share a few instances where I tend to avoid using them. This article focuses on JavaScript, but most of the concepts presented are language-neutral.

The debated example

In the comments of the aforementioned article, many of us started rewriting the following example if...else block.

const wow = arg => {

  if(arg === "dog"){
    return "LOVELY";
  } else if(arg === "cat"){
    return "CUTE";
  } else {
    return ("gimme an animal");
  }
}

wow("cat");
//-> "CUTE"

While the example was fine for demonstrating the author's point (we picked it apart anyway because we'll rip apart everything but our paychecks), it does present a few opportunities for improvement.

Else if, else if, else if

The first issue is that whenever a new condition is needed, a new else if clause must be added. So if you wanted to say "AWESOME" in response to "pony", you'd need to adjust the code as follows:

const wow = arg => {

  if(arg === "dog"){
    return "LOVELY";
  } else if(arg === "cat"){
    return "CUTE";
  } else if(arg === "pony"){
    return "AWESOME";
  } else {
    return ("gimme an animal");
  }
}

wow("pony");
//-> "AWESOME"

This would be repeated for every new animal and makes for very brittle, difficult to test, code.

The conditionals

Rather than using so many if...else if blocks, one could rewrite the function with conditional statements. Here is a comment from the linked article demonstrating this approach:

const wow = arg => (
  (arg === "dog" && "LOVELY") ||
  (arg === "cat" && "CUTE") ||
  "gimme an animal"
);

wow("cat");

There are no if statements present, but you are still left with the original maintenance issue. That is, you'd need to add an additional condition for each new animal.

The Data Map

One way to eliminate this growing set of else if statements is to store your relationships in a map. Consider the following:

const animals = {
  dog: "LOVELY",
  cat: "CUTE",
  pony: "AWESOME",
};

const wow = arg => {
  return animals.hasOwnProperty(arg) && animals[arg] || "gimme an animal";
};

wow("pony");
//-> "AWESOME"

Here, we've replaced the if...else statement with a lookup in a data map. With this, we have drastically simplified the wow function and we no longer need to modify it when a new animal comes along.

Before continuing, I'd like to point out that removing if statements is not the point here. The point is to make your code less brittle and easier to maintain. The latest iteration of this example could just as well have been written as follows:

const animals = {
  dog: "LOVELY",
  cat: "CUTE",
  pony: "AWESOME",
};

const wow = arg => {
  if(animals.hasOwnProperty(arg)){ //WTF if, who invited you?
    return animals[arg];
  }
  return "gimme an animal";
};

wow("pony");
//-> "AWESOME"

Going further...

You might look at the above and declare "But I still have to change the code! What's the difference?" I wouldn't fault you for that. So in this section, I will do a bit of restructuring in order to drive the point home.

First, let's abstract out the data.

//file: data.js

let animals;

//Let's pretend this is really being loaded from the database
//Let's also pretend the load is synchronous so we don't have
//get into a discussion of async/await or the Promise api
const loadAnimals = () => {  
  animals = {
    dog: "LOVELY",
    cat: "CUTE",
    pony: "AWESOME",
  };
};

const getAnimals = () => {
  if(!animals) loadAnimals();
  return animals;
};

export default getAnimals;

In this module, we are faking a database. The public getAnimals method will return the data from our datasource. Remember, the entire animals structure lives in the database, so modifications to it would happen there rather than in this file. For the sake of this discussion, let's pretend that data.js is the database.

Next, we implement our wow module.

//file: wow.js

import getAnimals from 'data';

const wow = name => {
  const animals = getAnimals();
  return animals.hasOwnProperty(name) && animals[name] || "I'm sorry Dave, I'm afraid I can't do that";
};

export default wow;

Notice here we import the data module and use it to grab the animals structure. Then, just as before, we either return the greeting (if one is present) or the silly string if no animal is found that matches the specified name.

The important point is that even if the set of animals changes or the greeting for each animal changes, this module does not need to be modified. That makes it much more maintainable since modifying or adding animals becomes an issue of data entry rather than a coding change. Your unit tests are greatly simplified because you need not test a branch per animal. In fact, you'd get 100% code coverage in this unit with just the following two tests.

• should accept a name and return a greeting for the specified animal.
• should return I'm sorry Dave, I'm afraid I can't do that if no animal matches; because all error messages should sound like a computer that sounds like a human trying to sound like a computer that sounds human.

Finally, you'd import and use this module from somewhere (here we'll just use index.js).

//file: index.js

import wow from 'wow';

wow('pony'); //-> AWESOME
wow('horse') //-> gimme an animal

Conclusion

Look, I'm not here to tell anyone how to code. I don't believe there is anything fundamentally wrong with using if statements. I absolutely don't believe in absolutes. I'm sure that last sentence harmed the same cat Schrödinger locked in that box. Did he ever answer to PETA for his actions?

Anyway, based on the needs of your project and your ability to convince the coding zealots you work with to turn a blind eye, you can likely get away with stringing a few if...else if...else statements together and shipping it. However, there are alternatives that will enhance the stability and testability of your code. This article points to the tip of that particular iceberg. If there is interest, I'll look into writing more about this and exploring some other popular patterns that can help. If not, just tell me to go to that place where that guy's cat was half of the time. Hell. I'm talking about hell.

Comments

[Alain Van Hout]

Very interesting and clear, thanks!

I’d argue that the second version of the data map solution (with the if rather than the ||) is the most maintainable, since it had the advantage of the data map, yet also has the defaulting behaviour as a separate part (rather than have it be clumped together with the happy-flow-case).

[Mahlon Gumbs]

Both of those are pretty much the same. The first takes advantage of short circuiting but the flow is the same. In this case, it comes down to preference for the most part.

I would not keep the precisely because it is data. Better to look that up or have it injected so that data changes don't result in coding modifications.

Thanks for your comment.

[Alain Van Hout]

Well, the compiled code (if there is such a thing) would be the same, but the denseness does differ a bit. Though whether the default value should be hardcoded, injected or looked up, that’s of course a different matter and will probably depend on business considerations (e.g. sometimes the default is, and can only be, a blank string, regardless of context).

[Mahlon Gumbs]

Sure. The example is a bit contrived. There would definitely be more error handling logic, as you've mentioned.

Help me understand what you mean by "denseness". From my pov, things look basically the same if you ignore the coding style.

const wow = arg => {
  return (
    animals.hasOwnProperty(arg) && animals[arg]
      || "gimme an animal";
  );
};

vs

const wow = arg => {
  if(animals.hasOwnProperty(arg)){ //WTF if, who invited you?
    return animals[arg];
  }
  return "gimme an animal";
};

[Alain Van Hout]

I’m mostly referring to the fact that it’s a single versus two separate statement/expressions, with the corrolary that the latter allows a line of whitespace inbetween to emphasize them being separate concerns. Moving the second part of the A || B to the next line aids the separation a bit, most people tend to not do that unless the total statement/line is too long to fit.

Edit: thanks for the interesting discussion btw

[Mahlon Gumbs]

Ah...ok. Understood. I agree.

edit: Absolutely. That's what this is all about. Thanks for the feedback.

[jakebman]

Is there a way to modify getAnimals() to change the default answer?

[Mahlon Gumbs]

Edit, I misread the question. Here is the corrected response.

Sure. As @Alain mentioned, you can use an optional parameter.

Optional Parameter

const loadAnimals = () => {  
  animals = {
    dog: "LOVELY",
    cat: "CUTE",
    pony: "AWESOME",
  };
};

const getAnimals = (overrides = {}) => {
  if(!animals) loadAnimals();
  return {
    ...animals,
    ...overrides,
  };
};

getAnimals({
  dog: "GREAT",
  monkey: "AGILE",
}); // -> {dog: "GREAT", cat: "CUTE", pony: "AWESOME", monkey: "AGILE"}

Thanks for your comment.

[Alain Van Hout]

It could be passed as a(n optional) second parameter.