A brief summary of Clean Code

Code should be as easy to read as a book. If you spend more than 5 seconds trying to understand a function it might be already doing more than it should.

In the book Clean Code: A Handbook of Agile Software Craftsmanship by Robert C. Martin the author talks about principles and techniques that works in practice, as opposed to what might work in theory.

It was written a long time ago, but a lot of it is still very relevant. In this post you will find a lot of things from the book and a bit more and how it could be applied to TypeScript.

Meaningful Names / Avoid mental mapping

Let the variable name tell you what it represents, don’t name it something that will make you constantly look up its definition to know what it is.

Can you spot the problem?

❌ Wrong

const d1 = d.getBd()
const d2 = d.getMd()

In the example above, you cannot really tell what d is and what getBd() and getMd() return. To find you you would have to find the definition and keep that information in mind. This is specially bad when you are a developer that didn’t originally write the code and have to make changes on it.

To avoid the mental mapping you can use meaningful names

✔️ Right

const birthDate = user.getBirthDate()
const mediumDate = user.getMediumCreatedDate()

This way there is not need to look it up, you know what they mean by just reading it.

Use intention revealing names

Make it clear what is the intention of your variable, don’t omit the intention.

Can you spot the problem?

❌ Wrong

const days = 30

Without context it doesn’t say much, it could be a lot of different things, instead you could reveal the intention

✔️ Right

const daysUntilNextYear = 30

This makes clear what the variable actually represents.

Avoid disinformation

Make it clean and precise what the variable/method is used for, don’t give it names that might mislead developers into thinking it might do something other than it says.

Can you spot the problem?

❌ Wrong

window.addEventListener('move', beforeMove)

The function to handle move is called beforeMove, but that is misleading, the event is triggered during move, not before.

Use a name that reflects what it is actually doing

✔️ Right

window.addEventListener('move', onMove)

This way the event name doesn’t mislead the developer into thinking it executes in a different situation.

Make meaningful distinction

Give variable names that make it more accurate, avoid creating variables that could have multiple meanings.

Can you spot the problem?

❌ Wrong

const money = 123

This variable without context doesn’t tell us exactly which currency it’s referring to, instead we can make it clear by making it more specific

✔️ Right

const moneyInUSD = 123

This way you can easily see the difference between this variable and other money variables, for example before a conversion it could be money in rupees.

Use pronounceable names

Use names that can be pronounced, this way it makes discussing the implementation with another developer much easier.

Can you spot the problem?

❌ Wrong

const srcImg = ''
const dstImg = ''

Imagine two developers pair programming, it would be awkward to talk saying unpronounceable names.

✔️ Right

const sourceImage = ''
const destinationImage = ''

By naming variables with pronounceable names it makes talking about it much easier, and even improves code readability.

Use searchable names

We often try to search the code for a piece of code that does something we are expecting, so by adding searchable names it makes it much easier to find things.

Can you spot the problem?

❌ Wrong

function renderFiter()

Typos are an issue too, you might be looking for renderFilter but if it’s misspelled you might not find it or have a hard time to find it. Be careful with typos and try to use the simplest term so another developer will try to search for the same term.

✔️ Right

function renderFilter()

This works because it’s correctly spelled and it’s using a simple term, so it’s easy to look for.

Hungarian notation

This is something that was needed decades ago because the compiler would use the prefix on the variable name to determine the type. Nowadays this is completely irrelevant.

Can you spot the problem?

❌ Wrong

const sName = ''

s in name doesn’t add any useful information for the developer and just adds unnecessary noise.

✔️ Right

const name = ''

Compilers nowadays don’t need the variable type in the name and there are tools to inform the developer the type of the variable they are using.

Use one word per concept

When you need to perform an operation, for example a GET request to an API you could write different verbs in your method name, but you should stick to a single word for that concept.

Can you spot the problem?

❌ Wrong

function getUsers()
function fetchCompanies()
function listColors()

In the example above it’s using 3 different terms to refer to the same thing, this makes searching harder and the code inconsistent. Instead you could pick one word and stick to it across your application.

✔️ Right

function fetchUsers()
function fetchCompanies()
function fetchColors()

This way it will be consistent, when you make and GET API call you can search for what you want using fetch###.

Add meaningful context

Context is important to determine the meaning of a variable, if you have a variable money without context you can’t really tell what is the currency.

Can you spot the problem?

❌ Wrong

const money = 123

There is not context to tell us what the currency is, this could be fixed by specifying more the variable name or wrapping the variable in a context.

✔️ Right

const credits = {
  currency: '$',
  money: 123,
}

This way both the currency and money are in the same context, so it’s easy to tell exactly how much that amount is.

Functions

Function should be as easy to read as a note, if you start having to “compile” the code in your head and read in different conditions then something is wrong.

Can you spot the problem?

❌ Wrong

function parseEffect(jump = false, rotate = true, fade = true) {
  if (jump && rotate) {
  } else if (jump && fade) {
  } else if (rotate && fade) {
  } else ...
}

This function has a lot of boolean parameters, which is going to increase the cyclomatic complexity of the function a lot, making it really hard to read. What could be done instead is break it in multiple smaller functions

✔️ Right

function parseJump()
function parseRotate()
function parseFade()

With smaller functions you can break a big problem in smaller problems, making it easier to understand and even implement.

Do one thing / Have no side effects

Functions should do a single thing to avoid causing side-effects (things that you don’t expect to happen in that function)

Can you spot the problem?

❌ Wrong

function saveUser(user) {
  database.start()
  database.users.save(user)
}

In the example above you cannot save an user without initializing a database connection, this might cause an unexpected behavior. Instead your function should handle one single task.

✔️ Right

function saveUser(user) {
  database.users.save(user)
}

This way there will be no side effects, the function is going to do only what it says it’s going to do.

Function parameters

The more parameters you have the more complex your function is going to be, you should always aim to keep a function as simple as possible.

Flag parameters

Flag parameters are specially bad because they create a new ramification of your function.

Can you spot the problem?

❌ Wrong

function showNotification(animate: boolean)

In the example above your function is doing two things: showing the notification without animation and with animation. Also, if you see this code

showNotification(true)

Can you immediately tell what it is doing, what it means? Your functions should tell, not ask.

✔️ Right

function showNotification()
function showNotificationAnimated()

This way it is very clear what the function is doing and in the middle of more code it makes the reading more fluid.

Functions with two/three parameters

Functions with multiple parameters increases the function cyclomatic complexity, it’s better to have functions tell instead of ask. Sometimes that’s required, for example:

✔️ Right

const point = new Point(x, y)

You need x and y to form a point.

Functions with object arguments

It’s ok to pass object parameters, in the end they represent one parameter each.

Can you spot the problem?

❌ Wrong

function distance(x1, y1, x2, y2)

In the example above I’m not using objects, I’m passing 4 parameters, but in reality I want to pass 2 parameters (points).

✔️ Right

interface Point { x: number; y: number }
function distance(point1: Point, point2: Point)

This way you logically group parameters and reduce the total of arguments your function has to handle.

Comments

Comments are sometimes ok, they can help understand edge cases, warn about consequences. In many other cases it might be just noise, you should always explain yourself in code.

Good: explain yourself in code

Make your code legible instead of adding comments to explain it.

Can you spot the problem?

❌ Wrong

// Check to see if the employee is eligible for full benefits
if ((employee.flags & HOURLY_FLAG) && (employee.age > 65))

The comment here is just noise, the code could explain itself.

✔️ Right

if (employee.isEligibleForFullBenefits())

By extracting the condition to a variable/function/method you can make the code tell you what it does.

Good: informative comments

Comments explaining something complex that cannot be changed, for example a regex.

Can you spot the problem?

❌ Wrong

const customUrlPattern = /\/([\w-]*-)?(\d+)/

This is a bit hard to read, a comment could explain it briefly.

✔️ Right

// matches /some-name-<id> url pattern
const customUrlPattern = /\/([\w-]*-)?(\d+)/

The comment here makes “reading/debugging” the regex easier, so the comment is not irrelevant.

Good: explanation of intent

Sometimes you might have a piece of code that doesn’t really say much by itself, but a comment might help explain why that is there.

Can you spot the problem?

❌ Wrong

setTimeout(updateScreen, 1000)

In the code above we can’t tell why the setTimeout is there just by reading the code. A comment would help developers understand why it is there.

✔️ Right

// we are delaying execution due to a temporary bug
// that is causing the screen to flicker
setTimeout(updateScreen, 1000)

This way you know why it is there.

Good: warning about consequences

Sometimes you might have a test that is skipping, or a piece of code that only executes under certain conditions and a comment to explain it would be helpful.

Can you spot the problem?

❌ Wrong

describe.skip('some test', () => {

In the example above there’s not explanation why the test is being skipped, one could think it’s a bug. A comment explaining why would be helpful.

✔️ Right

// this test takes 10m to execute, skipping on CI
describe.only('some test', () => {

This way it makes clear that the code is not an accident, it was intentionally put there for a reason and it warns about consequences of not skipping.

TODO comments

From my personal opinion: you should not use TODO comments, if there’s anything that needs to be done it should go on your task tracking system’s backlog. Otherwise in most cases it just gets stale and forgotten.

BAD: Noisy comments

Comments that don’t add any useful information.

Can you spot the problem?

❌ Wrong

// user id
const userId = 1

The variable name already states that it refers to the user id, there’s no need for a comment stating the same.

✔️ Right

const userId = 1

This is just more clean and already tells you what it means.

BAD: commenting code

Don’t comment code, just delete it.

Can you spot the problem?

❌ Wrong

// function sum(arr) {
//   return arr.reduce((a, b) => a + b)
// }

Commented code doesn’t do anything more than occupy space on your file. If you even need that you can go back on your version control (e.g. Git) and restore it.

✔️ Right

Delete unused code. It makes the code cleaner.

Principles and techniques

Some principles and techniques you can use.

Don’t repeat yourself

Also known as DRY, means that you shouldn’t have duplicated code. You can create a function and reuse it.

Can you spot the problem?

❌ Wrong

// home.tsx
if (type === 'announcement') {
  return (
    <div className="message-wrapper">...</div>
  )
}
// portal.tsx
if (type === 'announcement') {
  return (
    <div className="message-wrapper">...</div>
  )
}

A code that is doing the exact same things in two places is bad, you will have double the work to change, test, etc. This is known as WET (“write everything twice”, “we enjoy typing” or “waste everyone’s time”). Instead you could write a function and reuse in different places.

✔️ Right

// announcement.tsx
renderWarning() {
  if (type === 'announcement') {
    return (
      <div className="message-wrapper">...</div>
    )
  }
}
// home.tsx
renderWarning()
// portal.tsx
renderWarning()

This way you only have to maintain code in one place and any changes will be easier to implement.

Keep It Simple, Stupid (KISS)

Simple things are easier to maintain, easier for people to learn and use.

Avoid unnecessary complexity.

Albert Einstein once said something like “Make everything as simple as possible, but not simpler”.

LIFT

From Angular documentation:

Locate code quickly, Identify the code at a glance, keep the Flattest structure you can, and Try to be DRY

You Aren’t Gonna Need It (YAGNI)

Deleting code is a good thing. Keep it simple and don’t add unnecessary stuff.

Single Responsibility

Methods/functions should only be responsible for executing a single task.

Clean code is all about keeping the code simple, easy to read, easy to change.

Disclaimer: I’m using ❌ Wrong and ✔️ Right just to make it clearer what is the preferred approach, it’s not a rule that says that the approach is absolutely right or wrong.