DEV Community 👩‍💻👨‍💻

Cover image for Code Smell 156 - Implicit Else
Maxi Contieri
Maxi Contieri

Posted on • Originally published at maximilianocontieri.com

Code Smell 156 - Implicit Else

We learn if/else on our first programming day. Then we forget the else

TL;DR: Be explicit. Even with Else.

Problems

Solutions

  1. Write the explicit else

Context

If we early return on an IF sentence we can omit the else part.

Afterward, we Remove the IF and use polymorphism.

That is when we miss the real cases.

Sample Code

Wrong

function carBrandImplicit(model) {
  if (model === 'A4') {
    return 'audi';
  }
  return 'Mercedes-Benz';
}
Enter fullscreen mode Exit fullscreen mode

Right

function carBrandExplicit(model) {
  if (model === 'A4') {
    return 'audi';
  }
  if (model === 'AMG') {
    return 'Mercedes-Benz';
  }

  // Fail Fast
  throw new Exception('Model not found);
}
Enter fullscreen mode Exit fullscreen mode

Detection

[X] Automatic

We can check syntax trees and parse them and warn for missing else.

We can also rewrite them and perform mutation testing.

Tags

  • Conditionals

Conclusion

This kind of smell brings a lot of public debate, and hate.

We must exchange opinions and value each pros and cons.

Relations

More Info

Stop Using Implicit Else

When To Use Implicit Else

Credits

Photo by Elena Mozhvilo on Unsplash


The biggest issue on software teams is making sure everyone understands what everyone else is doing.

Martin Fowler


This article is part of the CodeSmell Series.

Top comments (4)

Collapse
lukeshiru profile image
Luke Shiru •

You really think throwing is preferable over an actual default case? From my point of view that second snippet is way more smellier than the first one. Having to wrap it on a try/catch in order to be "safe" makes it usage way less convenient.

My suggestion would be to just have something simple like this (added JSDocs to make DX even better):

const modelBrandMap = /** @type {const} */ ({
    A4: "Audi",
    AMG: "Mercedes-Benz",
});

/**
 * @template {keyof typeof modelBrandMap} Model
 * @param {Model} model
 */
const carBrand = model => modelBrandMap[model];
Enter fullscreen mode Exit fullscreen mode

And then the usage is as simple as:

// We can graceful default the brand to "Unknown":

console.log(`The brand for "${model}" is "${carBrand(model) ?? "Unknown"}".`);

// Or if you prefer to show an error, you can always:

const brand = carBrand(model);

brand
    ? console.log(`The brand for "${model}" is "${brand}"`)
    : console.error(`There's no brand for "${model}"`);

// Or maybe using if:

const brand = carBrand(model);

if (brand) {
    console.log(`The brand for "${model}" is "${brand}"`);
} else {
    console.error(`There's no brand for "${model}"`);
}
Enter fullscreen mode Exit fullscreen mode

As you can see, we have several options, none of which will make the app explode for not finding a result. Now with your approach we have less options and we need to use more boilerplate:

// The graceful approach:

try {
    console.log(`The brand for "${model}" is "${carBrandExplicit(model)}".`