Maxi Contieri

Posted on Aug 6 • Originally published at maximilianocontieri.com

Code Smell 156 - Implicit Else

#javascript #webdev #beginners #programming

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

TL;DR: Be explicit. Even with Else.

Problems

Readability
Cognitive Load
Unforeseen conditions
Fail Fast Principle violation

Solutions

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';
}

Right

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

  // Fail Fast
  throw new Exception('Model not found);
}

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.

Conclusion

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

We must exchange opinions and value each pros and cons.

Relations

Code Smell 102 - Arrow Code

Maxi Contieri ・ Nov 15 '21 ・ 2 min read

#oop #javascript #cleancode #refactoring

Code Smell 36 - Switch/case/elseif/else/if statements

Maxi Contieri ・ Nov 28 '20 ・ 1 min read

#oop #webdev #tutorial #codenewbie

More Info

Stop Using Implicit Else

When To Use Implicit Else

Fail Fast

Maxi Contieri ・ Dec 6 '20 ・ 6 min read

#codenewbie #programming #webdev #oop

How to Get Rid of Annoying IFs Forever

Maxi Contieri ・ Nov 9 '20 ・ 5 min read

#oop #programming #codenewbie #tutorial

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

Software Engineering Great Quotes

Maxi Contieri ・ Dec 28 '20 ・ 13 min read

#codenewbie #programming #quotes #software

This article is part of the CodeSmell Series.

How to Find the Stinky parts of your Code

Maxi Contieri ・ May 21 '21 ・ 7 min read

#codenewbie #tutorial #codequality #beginners

Top comments (4)

Luke Shiru • Aug 12

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];

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}"`);
}

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)}".`);
} catch {
    console.log(`The brand for "${model}" is "Unknown".`);
}

// Or maybe you want to reduce duplication of code, so yo do something like (which still isn't pretty):

let brand = "Unknown";
try {
    brand = carBrandExplicit(model);
} catch {}

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

// If we prefer to show the error:

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

But basically less options and more boilerplate just because you took the nuclear option of throwing. Take a look at native methods such as Array.prototype.find: When it finds an item that matches the predicate function, it returns it, but if it doesn't find it, it doesn't throw, instead it just returns undefined. The developer already knows that if the value received is undefined, it means the item wasn't found, so no need to throw for that. In TypeScript or JS with type check enabled you get the type TypeOfItem | undefined from it, so you know you need to check for undefined or use ?? to be safe. Typing for errors is way more nasty (you need to use asserts to create invariants for every function that throws).

We can go simpler than that, with any function and operation that returns a number, if an operation to get a number is invalid, you get the infamous NaN, which is still not throwing. Developers really don't want to have to wrap every single function call in a try/catch block, so you really should avoid that as a practice, even if it is "good" in other languages.

Cheers!