Category: JavaScript

Bad code has lots of unique characters. In this article, we’ll look at each one and what they are. We look at more general code smells.

---

Too Much Information

Well-defined modules should be small and allow us to do a lot with little code. They don’t offer many functions to depend upon, so coupling is loose.

A bad module has lots of functions that we have to call to get something done, so coupling is high.

Things that are exposed to modules should be minimized. Data and implementation should be hidden except when they can’t be. We shouldn’t create lots of instance variables that are inherited by other classes.

Utility functions should be hidden from other modules. In the end, we should be able to reference a few things to do a lot to minimize coupling.

---

Dead Code

Dead code should definitely be removed from our codebase.

It’s ignored and it’s never updated to follow the latest conventions. Therefore, it should be removed from our code. Our tests should check if it works without the dead code.

---

Vertical Separation

Variables and functions should be closed to where they’re used so that we don’t have to do lots of scrolling to trace our code.

---

Inconsistency

We should be consistent with our naming. For example, we should stick to JavaScript conventions when naming things.

Functions and classes are upper camel case. Variables are camel case. Constants are upper case. Consistently named things are easier to modify.

---

Clutter

We should remove anything that clutters our code. Useless code should be removed. Unused entities should be gone.

Everything should be well-organized, clean, and free of clutter.

---

Artificial Coupling

Coupling should always be minimized so things that shouldn’t be coupled together stay uncoupled. The less one thing has to know about another, the better.

They serve no purpose and making changes is harder because we have to deal with all the coupling whenever we make changes. When we get something to work, we should clean up our code so that we don’t have these situations.

---

Feature Envy

The methods of a class should be interested in the variables and functions of the class they are in, and not the ones from other classes.

We should reference as little code from external sources as possible.

The following example shows what feature envy is:

We have the ShapeCalculator class that references the Rectangle class a lot. We call its constructor and instance variables.

However, we shouldn’t do this because it’s referencing too much from the Rectangle class. We can remove references to the instance variables as follows:

As we can see, we didn’t have to touch the internals to get the area of a rectangle. It’s much better to not reference the length and width from a Rectangle instance if we don’t have to.

This is because when we make changes to the Rectangle class, we have to change a lot of things in the ShapeCalculator class if we do reference these entities.

For example, when we change the names of length and width to something else, then we have to change them everywhere.

---

Selector Arguments

Boolean arguments for selecting functionality in a function is bad. It’s hard to know what true or false means when we select it. We should split a function into two if we do need a selector argument. It’s just as bad as any other argument.

Avoiding boolean arguments is one of the clear-cut criteria to separate a function into multiple functions. If we do need selectors, they should be something clearer like strings or integers.

---

Obscured Intent

We want our code to be as clear to our readers as possible. Therefore, naming should reveal the intent of identifiers.

So, variable x is a bad name since it tells us nothing, but numApples is good since we know what it means. We know that it stores a number of apples.

---

Misplaced Responsibility

Code should be placed where we expect it to be. For example, PI should belong to a Math class as a constant. We shouldn’t be clever about where to put certain functionalities. The place we put it should be intuitive to the reader.

Names of functions should tell us where to put our code. For instance, getHoursWorked should be in the Employee class because it belongs to an employee.

---

Inappropriate Static

Static methods should only be used on functions that don’t operate on an instance. So, getHoursWorked shouldn’t be a static method in the Employee class, since the number of hours worked belongs to the employee.

Something that’s suitable for static methods is the ones that don’t care about the instance of the class it operates on.

For example, the Math.min method should be a static method because we don’t need the Math instance for anything.

---

Conclusion

When writing clean code, we have to think about many things. However, most of them are common sense. We should write code that is clear and expose as little to the outside as possible to reduce coupling.

Names should be clear so everyone knows what we mean. Finally, things have to placed where they make sense.

Bad code has lots of unique characters. In this article, we’ll look at each one and what they are. We look at more general code smells.

Use Explanatory Variables

Variables should have names that explain themselves. For example, something that isn’t explanatory is something like x or y. We don’t know what they mean.

On the other hand, numOranges or numApples are explanatory since they tell us what we’re going to store in these variables. We know that we’re setting it to the number of oranges and apples respectively.

Function Names Should Say What They Do

Function names need to tell what they’re doing so we don’t have to guess.

For example, we don’t know what date.add(2) does? It can be adding seconds, minutes, hours, days, months, or whatever we didn’t think of yet.

We should rename it to something more clear like addDays or addMonths so that we know what we’re adding.

If we have to look at the code or documentation to know what it does at a high level, then maybe it should be renamed.

Understand the Algorithm

We should understand the code that we write. Otherwise, we may hit a fluke once in a while, but if we don’t know what it’s doing exactly, then we’re going to run into problems eventually.

When we do run into problems, we won’t know how to resolve them since we didn’t understand what we wrote in the first place.

Also, writing code by guessing creates messy code as we’re messing with them to get them to work but we’re afraid that when we clean up the code, that it’ll fail again.

Therefore, we should think and understand before and during the time that we write our code.

Prefer Polymorphism to Conditionals

Conditionals are long messy. Nested ones are even worse. We should use them as little as possible if we’re just going to use them to invoke different objects.

Follow Standard Convention

Everyone should follow coding standards based on industry norms. In JavaScript, there’re conventions for naming variables, constants, and functions.

Also, the spacing and maximum line length are standardized across files.

We can deal with these automatically by using Linters and code formatters.

Other things like vertical formatting and function and variables placement have to be dealt with manually.

Replacing Magic Numbers with Named Constants

It’s hard to know what a number means when it’s not assigned to a constant.

Therefore, if we use a number as a constant, we should assign it to one so that we know what they mean.

For example, if we have a constant for hours per day, we should write:

const HOURS_PER_DAY = 24;

instead of just 24.

Other issues include floating-point numbers that need precision. To keep the precision the same, we should assign them to a constant.

Something like PI and E should be assigned to constants so that they always have the same precision.

In addition to numbers, they also apply to any other constant values that are repeatedly used. For example, if we always write tests using the string 'Joe', then we can assign it to a constant and reference it everywhere.

This way, we avoid errors typing and reduce the chance of creating bugs.

Be Precise

We should be precise with everything in our code. For example, we shouldn’t use the word array in our variable names unless it’s an array.

If we expect something to return null or undefined, then we should check for those.

Also, we should expect the first match of anything to be the correct match. We should actually check for the conditions that we’re looking for.

Structure over Convention

We should enforce structure over convention. We can shape the structure with tests and reviews.

Encapsulate Conditionals

When we have a conditional with multiple conditions, consider encapsulating them in a function.

For example, instead of writing:

if (employee.type === 'FULL_TIME' && employee.hasHealthBenefits) {

}

We can put the boolean expression into a function as follows:

const hasFullTimeHealthBenefits = (employee) => {
  return employee.type === 'FULL_TIME' && employee.hasHealthBenefits;
}

if (hasFullTimeHealthBenefits(employee)) {

}

Avoid Negative Conditionals

Negatives are hard on our brains, so we should use positive boolean expressions whenever we can. For example:

if (isEmployed) {

}

is better than:

if (!isNotEmployed) {

}

Functions Should Do One Thing

Functions should do only one thing. If a function does multiple things, then we should split it into smaller functions.

For example, if we have the following code:

const calculateTotalPay = (employees) => {
  let totalPay = 0;
  for (let employee of employees) {
    totalPay += employee.regularPay;
    totalPay += employee.overtimePay;
    totalPay += employee.bonus;
  }
  return totalPay;
}

We can instead move the totalPay calculations to its own function as follows:

const calculateEmployeePay = (employee) => {
  return employee.regularPay +
    employee.overtimePay +
    employee.bonus;
}

const calculateTotalPay = (employees) => {
  let totalPay = 0;
  for (let employee of employees) {
    totalPay += calculateEmployeePay(employee);
  }
  return totalPay;
}

Now we have one function to get the total pay and employee’s pay instead of one big function to get both the employee’s pay and the total pay of all employees.

Conclusion

We should follow standard conventions when writing code. Names should be clear, they should also follow the same case.

Double negatives are also hard to understand, so we should avoid them.

We should assign any literal values to constants if they’re used repeatedly.

Finally, functions should only do one thing to make them simple.

ad code has lots of unique characteristics. In this article, we’ll look at each one and what they are. We look at writing comments, functions, and general code smells and heuristics.

Comments

Inappropriate Information

Information that shouldn’t be in the comments like author and changelogs are in the comments. They should be in source control systems, bug trackers, and other record-keeping systems.

Change histories should be in source control systems for example. It has metadata for authors, code changes, change date, etc. These shouldn’t be in the comments.

Comments should be for technical notes about the code.

Obsolete Comment

Comments that are old, irrelevant, or wrong are misleading. They get old quickly. The code should be clean enough to not need so many comments.

They become outdated quickly, so they should be avoided.

Redundant Comment

If the code adequately explains itself, then we don’t need comments explaining it. JSDoc that says nothing more than the signature isn’t also very useful.

They should say things that can’t be shown by the code.

Poorly Written Comments

Comments that are worth writing should be written well. We should make sure that they’re the best comments that we can write.

Commented-Out Code

Commented out code can be misleading. Why are they still there if they’re commented out?

We should delete the code if they aren’t needed. They can also be reverted from the source control system’s change record.

Environment

Build Requiring More Than One Step

Builds shouldn’t require more than one step. The more things we have to do manually, the worse that everyone suffers.

We shouldn’t have to do manual operations like checking code out from source control or run a bunch of commands and scripts every time we have to run a build.

There’re so many build pipeline solutions that the button should be a one-click process.

Tests Requiring More Than One Step

Running tests should also be easy. All tests should be run with one command. Either we can run commands with one click on an IDE or by typing in one command.

Functions

Too Many Arguments

Functions should have a few arguments as possible. No arguments are best. More than 3 is questionable.

Output Arguments

We shouldn’t be returning arguments straight at the end of the function as-is. It just makes no sense.

Flag Arguments

Flag arguments mean that a function does more than one thing, so they should be eliminated.

Dead Function

Functions that aren’t called should be removed. Dead code takes up space and misleads people. We can always get it back from source control history.

General

Multiple Languages in One Source File

One file should only be one language. The more language is in a file, the more confusing it is.

Clean separation of logic and markup is always good. JSX is just a different syntax for JavaScript, so it’s actually one language.

Obvious Behavior That’s Unimplemented

Obvious behavior should be implemented. If it’s so useless that it can stay unimplemented, then we can probably eliminate it.

We should implement obvious functionality as described by a function so that the name isn’t misleading.

Incorrect Behavior at the Boundaries

Developers often trust their intuition when they write their functions and think that everything works. We often ignore corner and boundary cases.

We should check our code by writing tests for these conditions and don’t assume that it’ll work properly with them.

Overriding Safety Mechanism

Safety mechanisms in our code shouldn’t be overridden when we write code. Risky changes should be minimized. The end might be lots of bugs and lots of debugging.

Turning off failing tests are bad and we should think about the possible consequences when we do.

Duplication

Code duplication is bad. Every time we have to change duplicate code, we have to change them in multiple places.

We can remove them by abstracting the code and putting them in a central location.

Coding becomes faster and less error-prone since we only have to change things in one place.

The most obvious form is clumps of identical code.

Another form is conditional statements that appear multiple times in different parts of the code. We can replace them with polymorphic code.

Most design patterns are well-known ways to eliminate duplication. They’re structured to eliminate them.

Code at Wrong Level of Abstraction

When we abstract code, we should make them completely. The separation is complete. All the higher-level concepts to be in the base class.

Constants, variables, and utility functions shouldn’t be in the base class.

Source files, components, and modules should at a higher level of abstraction also be separated from ones with lower levels of abstraction.

We don’t wrong higher-level and lower-level code mixed together.

For example, if we have an Account class:

class Account {  
  saveAccountToDb() {}  
  getAccount() {}  
  setAccount() {}  
}

Then we have code at 2 levels of abstraction because we have saveAccountToDb which is a helper method to save data to the database.

We want to move it to a helper class or function.

Conclusion

Comments should be minimized and are useful and up to date when they’re there.

Functions should have as few arguments as possible and only do one thing. This also means we shouldn’t have flag arguments.

When we write code, we should check for boundary and corner cases to avoid bugs. Also, we should override safety features like removing important tests. They’re probably there for a reason.

Finally, code duplication is bad.

Classes in JavaScript is syntactic sugar on top of the prototypical inheritance features of the language. However, in terms of writing clean code, the principles still apply since they have the same structure as classes in class-based languages.

In this article, we’ll look at how to write JavaScript classes in a clean and maintainable way.

We’ll look at how to organize for changes and using the class syntax instead of using constructor functions.

Organizing for Change

We have to prepare for classes to be changed when we organize them. This means that we should make them extendible rather than having to constantly modify code to get the functionality we want in our class.

Our methods should be simple. Simple methods are easier to test and we don’t have to change them as much.

We should follow the open/closed principle, which states that a piece of code should be open for extension but closed for modification.

This applies to classes just like another piece of code.

For example, if we have the following Rectangle class:

class Rectangle {
  constructor(length, width) {
    this.length = length;
    this.width = width;
  }

  get area() {
    return this.length * this.width;
  }
}

Then we can easily add a getter method for computing the perimeter of a rectangle as follows:

class Rectangle {
  constructor(length, width) {
    this.length = length;
    this.width = width;
  }

  get area() {
    return this.length * this.width;
  }

  get perimeter() {
    return 2 * (this.length + this.width);
  }
}

As we can see, we didn’t have to modify the existing code to add a method for computing the perimeter. We just add the perimeter getter method and be done with it.

Use the Class Syntax Instead of Constructor Functions

It’s time to move on to the class syntax instead of using constructor functions.

We can see why with the following example of inheritance:

function Person(name, age) {
  this.name = name;
  this.age = age;
}

function Employee(name, age, employeeCode) {
  Person.call(this, name, age);
  Employee.prototype.constructor = Person;
  this.employeeCode = employeeCode;
}

First, we have to create the Person constructor, then to make Employee ‘s prototype Person and set all the inherited properties, we have to first write:

Person.call(this, name, age);

to set all the instance variables, and:

Employee.prototype.constructor = Person;

to set the Employee’s prototype constructor to Person . We can easily miss any of these 2 lines and the Employee constructor won’t be inheriting from the Person constructor.

If we create an Employee instance as follows:

const employee = new Employee('Joe', 20, 'waiter');

Then we should see something like the following under the __proto__ property:

constructor: _ƒ Person(name, age)_

This means that we set the prototype of the Employee instance to the Person constructor correctly.

With the class syntax, we only have to use the extends keyword to inherit from one class. We can rewrite the code above as follows:

class Person {
  constructor(name, age) {
    this.name = name;
    this.age = age;
  }
}

class Employee extends Person{
  constructor(name, age, employeeCode) {
    super(name, age);
    this.employeeCode = employeeCode;
  }
}

Then when we create the same Employee instance as follows:

const employee = new Employee('Joe', 20, 'waiter');

Then we should see something like the following under the __proto__ property:

constructor: _class Employee_

As we can see, both console.log outputs are the same, except for the function and class difference, but they’re the same since classes are the same as constructor functions.

However, we don’t have to use call or this , and set the variables of the superclass manually.

The JavaScript interpreter will tell us if we forgot to call super or use the extends keyword.

There’s no going back to the old constructor function syntax nowadays since it’s pretty inconvenient.

Conclusion

When we design classes, we should organize for change. This means that we should have code that’s open for extension but closed for modification.

This reduces the risk of messing up existing code why allowing us to keep making changes by adding new code.

Also, it’s time to move on to the class syntax for creating constructor functions. It’s hard to do inheritance with old constructor functions, while the class syntax makes everything much easier.

Concurrency is an important part of most modern programs. To achieve this in JavaScript, we have to use asynchronous code, which is non-blocking.

In this article, we’ll look at how to write asynchronous code in a way that’s clean and easy to read and change.

---

Use Promises Instead of Callbacks

Promises have been a standard object since ES6, so the previous asynchronous callbacks should all be replaced with promises.

Using callbacks is a real pain if we have any sequential code since we have to nest them on multiple levels.

For example, if we want to run multiple setTimeout callbacks without promises, then we have to nest them as follows:

setTimeout(() => {
  console.log('foo');
  setTimeout(() => {
    console.log('bar');
    setTimeout(() => {
      console.log('baz');
    }, 200)
  }, 200)
}, 200)

As we can see, we only have three callbacks and the nesting is already very ugly. We have to clean this up so that this is more pleasant to look at and easier to understand.

We can do this with promises as follows:

const timeOutPromise = (str) => {
  return new Promise(resolve => {
    setTimeout(() => {
      resolve(str);
    }, 200)
  })
}
timeOutPromise('foo')
  .then((val) => {
    console.log(val);
    return timeOutPromise('bar');
  })
  .then((val) => {
    console.log(val);
    return timeOutPromise('baz');
  })
  .then((val) => {
    console.log(val);
  })

As we can see, with promises, we can chain them with the then method with a callback passed into it. We don’t have to nest callbacks except in the timeoutPromise function, and it’s only two levels of nesting instead of three or more.

We get the resolves value of a promise in the parameter of the callback that we pass into the then method.

To catch errors, we can use the catch method with a callback passed in as follows:

const timeOutPromise = (str) => {
  return new Promise(resolve => {
    setTimeout(() => {
      resolve(str);
    }, 200)
  })
}
timeOutPromise('foo')
  .then((val) => {
    console.log(val);
    return timeOutPromise('bar');
  })
  .then((val) => {
    console.log(val);
    return timeOutPromise('baz');
  })
  .then((val) => {
    console.log(val);
  })
  .catch((err) => console.error(err))

---

Async/Await Is a Cleaner Syntax for Chaining Promises

ES2017 introduced the async and await syntax, which is a cleaner way of chaining promises.

We can rewrite what we had above by writing:

const timeOutPromise = (str) => {
  return new Promise(resolve => {
    setTimeout(() => {
      resolve(str);
    }, 200)
  })
}

(async () => {
  let val;
  val = await timeOutPromise('foo');
  console.log(val);
  val = await timeOutPromise('bar');
  console.log(val);
  val = await timeOutPromise('baz');
  console.log(val);
})();

It’s exactly the same as:

const timeOutPromise = (str) => {
  return new Promise(resolve => {
    setTimeout(() => {
      resolve(str);
    }, 200)
  })
}

timeOutPromise('foo')
  .then((val) => {
    console.log(val);
    return timeOutPromise('bar');
  })
  .then((val) => {
    console.log(val);
    return timeOutPromise('baz');
  })
  .then((val) => {
    console.log(val);
  })

The one difference is that the resolved value is assigned to val via the assignment operator. This assignment works as long as we have await before our promises.

To handle rejected promises, we can use the try...catch clause as we do with synchronous code:

const timeOutPromise = (str) => {
  return new Promise(resolve => {
    setTimeout(() => {
      resolve(str);
    }, 200)
  })
}

(async () => {
  try {
    let val;
    val = await timeOutPromise('foo');
    console.log(val);
    val = await timeOutPromise('bar');
    console.log(val);
    val = await timeOutPromise('baz');
    console.log(val);
  } catch (err) {
    console.error(err);
  }
})();

async functions only return promises, so we can’t use them as general-purpose functions. They’re syntactic sugar for promises and not a replacement of it.

---

Conclusion

To write asynchronous code, promises are the way to go. They let us chain multiple of them together without nesting callbacks.

We should convert asynchronous code to promises if they aren’t returned as promises already. To chain them, we can use the then method.

To catch errors from rejected promises, we can use the catch method.

async and await are syntactic sugar for promises. They’re the same thing but async and await is shorter.

We can use try...catch to catch errors from rejected promises with async and await.