Clean Up Code Smells with Clean Code: TypeScript Edition

Common Code Smells in JavaScript and TypeScript and How To Fix Them

What is a Code Smell?

According to Wikipedia, in computer programming, a code smell is any characteristic in the source code of a program that possibly indicates a deeper problem.

To simplify the definition, code smells are a result of unclean or misguided programming.

Code smells have many severities, from minor code smells that maybe acceptable, to code smells that make program development becomes much more complicated and expensive.

Code Smell in JavaScript and TypeScript

Although some of the code smells are universal like:

• Too much comments
• Too much of “TODO”
• Too many parameters on function
• …..

In this article I will only focus on Code Smells that are common in JavaScript and TypeScript.

Redundant casts and non-null assertions should be avoided

The TypeScript compiler automatically casts a variable to the relevant type inside conditionals where it is possible to infer the type (because typeof, instanceof, etc was used). This compiler feature makes casts and not-null assertions unnecessary.

Should avoid:

function getName(x?: string | UserName) {
  if (x) {
    console.log("Getting name for " + x!); // Noncompliant

    if (typeof x === "string")
      return (x as string); // Noncompliant
    else
      return (x as UserName).name; // Noncompliant
  }
  return "NoName";
}

Do this instead:

function getName(x?: string | UserName) {
  if (x) {
    console.log("Getting name for " + x);

    if (typeof x === "string")
      return x;
    else
      return x.name;
  }
  return "NoName";
}

Ternary operators should not be nested

Ternary operators are widely used in JavaScript because of its simplicity and easy to read. But sometimes we abused it too much. Instead, we should use another line to express the nested operation as a separate statement.

Should avoid:

function getReadableStatus(job) {
  return job.isRunning() ? "Running" : job.hasErrors() ? "Failed" : "Succeeded ";  // Noncompliant
}

Do this instead:

function getReadableStatus(job) {
  if (job.isRunning()) {
    return "Running";
  }
  return job.hasErrors() ? "Failed" : "Succeeded";
}

Array-mutating methods should not be used misleadingly

Many of JavaScript’s Array methods return an altered version of the array while leaving the source array intact. reverse and sort do not fall into this category. Instead, they alter the source array in addition to returning the altered version, which is likely not what was intended.

This rule raises an issue when the return values of these methods are assigned, which could lead maintainers to overlook the fact that the original value is altered.

Should avoid:

var b = a.reverse(); // Noncompliant
var d = c.sort(); // Noncompliant

Do this instead:

var b = [...a].reverse();
a.reverse();

c.sort(); // this sorts array in place

Assignments should not be redundant

The transitive property says that if a == b and b == c, then a == c. In such cases, there’s no point in assigning a to c or vice versa because they’re already equivalent.

This rule raises an issue when an assignment is useless because the assigned-to variable already holds the value on all execution paths.

Should avoid:

a = b;
c = a;
b = c; // Noncompliant: c and b are already the same

Do this instead:

a = b;
c = a;

Do not abuse “Any” in TypeScript

Nothing much to say about this, you should only use “any” on legacy libraries that aren’t type safe, in 99% of cases we can avoid “any” to have type safe

Type aliases should be used

Union and intersection types are convenient but can make code harder to read and maintain. So if a particular union or intersection is used in multiple places, the use of a type alias is recommended.

Should avoid:

function foo(x:string|null|number) { // Noncompliant
  // ...
}
function bar(x:string|null|number) {
  // ...
}
function zoo(): string|null|number {
  return null;
}

Do this instead:

type MyType = string | null | number;

function foo(x: MyType) {
  // ...
}
function bar(x: MyType) {
  // ...
}
function zoo(): MyType {
  return null;
}

“undefined” should not be passed as the value of optional parameters

Unlike in JavaScript, where every parameter can be omitted, in TypeScript you need to explicitly declare this in the function signature. Either you add ? in the parameter declaration and undefined will be automatically applied to this parameter. Or you add an initializer with a default value in the parameter declaration. In the latter case, when passing undefined for such parameter, default value will be applied as well. So it’s better to avoid passing undefined value to an optional or default parameter because it creates more confusion than it brings clarity. Note, that this rule is only applied to the last arguments in function call.

Should avoid:

function foo(x: number, y: string = "default", z?: number) {
  // ...
}

foo(42, undefined); // Noncompliant
foo(42, undefined, undefined); // Noncompliant
foo(42, undefined, 5); // OK, there is no other way to force default value for second parameter

Do this instead:

function foo(x: number, y: string = "default", z?: number) {
  // ...
}

foo(42);

Regular expressions should not contain multiple spaces

Multiple spaces in a regular expression can make it hard to tell how many spaces should be matched. It’s more readable to use only one space and then indicate with a quantifier how many spaces are expected.

Should avoid:

const pattern = /Hello,   world!/;

Do this instead:

const pattern = /Hello, {3}world!/;

“delete” should not be used on arrays

The delete operator can be used to remove a property from any object. Arrays are objects, so the delete operator can be used here too, but if it is, a hole will be left in the array because the indexes/keys won’t be shifted to reflect the deletion.

The proper method for removing an element at a certain index would be:

• Array.prototype.splice - add/remove elements from the array
• Array.prototype.pop - add/remove elements from the end of the array
• Array.prototype.shift - add/remove elements from the beginning of the array

Should avoid:

var myArray = ['a', 'b', 'c', 'd'];

delete myArray[2];  // Noncompliant. myArray => ['a', 'b', undefined, 'd']
console.log(myArray[2]); // expected value was 'd' but output is undefined

Do this instead:

var myArray = ['a', 'b', 'c', 'd'];

// removes 1 element from index 2
removed = myArray.splice(2, 1);  // myArray => ['a', 'b', 'd']
console.log(myArray[2]); // outputs 'd'

Imports from the same modules should be merged

Multiple imports from the same module should be merged together to improve readability.

Should avoid:

import { B1 } from 'b';
import { B2 } from 'b'; // Noncompliant

Do this instead:

import { B1, B2 } from 'b';

Conclusion

There are a lot of minor code smells in JavaScript or TypeScript that I couldn’t list them all, just selected major code smells.
If you want to know more about code smells in TypeScript, check out this page

Last Words

Although my content is free for everyone, but if you find this article helpful, you can buy me a coffee here

Build apps with reusable components like Lego

Bit’s open-source tool help 250,000+ devs to build apps with components.

Turn any UI, feature, or page into a reusable component — and share it across your applications. It’s easier to collaborate and build faster.

→ Learn more

Split apps into components to make app development easier, and enjoy the best experience for the workflows you want:

→ Micro-Frontends

→ Design System

→ Code-Sharing and reuse

→ Monorepo

Learn more

How We Build Micro Frontends

How we Build a Component Design System

How to reuse React components across your projects

5 Ways to Build a React Monorepo

How to Create a Composable React App with Bit