In this series, I’ll talk about common refactors I suggest when doing code review in my day to day. I’ll start with conditionals, including severals example refactors. The goal is to help produce code that’s easier to read and understand, and thus, easier to maintain and produces fewer bugs.

These examples will be in TypeScript but the refactors are common patterns and can apply to most languages out there, such as Python, Ruby, Swift, PHP, Java, Go, JavaScript, etc.

Unnecessary Indentation: Early Return

Unnecessary indentation is not related only to conditionals, but that’s one of the places where it shows up quite often.

Consider this code:

if (something) {
  return true;
} else {
  return false;
}

There’s nothing wrong with the code, but one of the properties of good code is that it’s easy to read and understand. With that in mind, understanding two related branches (if-else) is harder than understanding two independent branches (if-if).

Let me explain 🙂

if (something) {
  return true;
} else {
  if (somethingElse) {
    return true;
  } else {
    return false;
  }
}

To understand that code, you need to read both sides of the if conditional, and know that the if (somethingElse) conditional will only be executed if something is not truthy.

This means that the second if statement depends on the condition of the first if statement. That ties them together, makes the code more complex, and thus, harder to understand.

Sometimes, there’s no easy refactor, and that’s fine. Rules are not absolute, but for this case in particular, consider this:

if (something) return true;

if (somethingElse) {
  return true;
} else {
  return false;
}

Now the conditionals are not tied together. They are independent, so we can read and understand them separately.

We can still do better, though:

if (something) return true;
if (somethingElse) return true;

return false;

Now we reduced two nested if-else conditionals to two simple if conditionals.

Using the “Early Return” refactor can help make your conditional logic much simpler, which should be your goal.

Side Note: Not only conditionals

Conditionals are not the only place where you want to keep an eye out for unnecessary indentation levels. Another common place unnecessary indentation happens are loops and iterations, such as for statements:

function calculateOrderTotal(items: OrderItem[]): number {
  let total = 0;
  for (const item of items) {
    if (item.discount > 0) {
      const discountAmount = item.price * item.discount;
      total += item.price - discountAmount;
    } else {
      total += item.price;
    }
  }
  return total;
}

This could be refactored to:

class OrderItem {
  // ...
  getPrice() {
    if (this.discount === 0) return this.price;

    const discountAmount = this.price * this.discount;
    return this.price - discountAmount;
  }
} 

function calculateOrderTotal(items: OrderItem[]): number {
  let total = 0;
  for (const item of items) {
    total += item.getPrice();
  }
  return total;
}

We moved the nested conditional that was dependent on the for statement into the OrderItem.getPrice() method. The conditional inside that method doesn’t need to concern itself with being inside a for statement, it can be understood independently.

In this case, the OrderItem object was “leaking logic”. In other words, logic that should live inside that object was being handled outside of it. If your codebase was functional, then this would likely be extracted into a getPrice(item) function instead, and it could live in a place where you keep all the functions that operate on the OrderItem data type.

Positive Conditions

It’s always easier to reason about positive statements than negative ones. This is because the negatives require the extra step of applying a negation.

The sentence: “If it’s sunny, I’ll go to the movies” is easier to understand than “Unless it’s not raining I won’t go to the movies”.

The same happens with code:

if (sunny) {
  goToMovies()
}

That positive should be preferred over this, negative:

if (!raining) {
  goToMovies()
}

DeMorgan’s Law

DeMorgan’s Law is a set of two rules we can apply to transform a boolean operation (conditional logic) from one form (AND) to another (OR):

The rule says:

!(A || B) = !A && !B
!(A && B) = !A || !B

We swap the operator (if it was an AND then we change it to an OR, and vice-versa) and apply the negation to each part rather than as a group.

This gives us an easy, mechanical way to remove negation from groups of conditions:

if (!(isRaining || !haveMoney)) {
  goToMovies()
}

It surely doesn’t seem easy to understand, right? We have a group, negated, and then inside the negated group we have another negation in !haveMoney!

We can get rid of the outer negation by applying a rule and swapping the && for an || like so:

if (!isRaining && haveMoney) {
  goToMovies()
}

Notice the actual refactor would make us write !!haveMoney which is the exact same as haveMoney because the double negation cancels itself out.

This rule, together with “Naming Conditionals” has helped me make sense of several monstrous conditionals in my career!

Explicit Comparisons

Explicit is better than implicit.

The Zen of Python

When reading code, explicit code is easier to understand. Consider the snippet below:

if (!person) {
  doSomething();
}

Now consider this one, which is more explicit:

if (person !== null) {
  doSomething();
}

They are both quite simple and easy to understand, but the second one gives you more information: doSomething will only be called when person is not null. It’s a strict comparison and gives us more context.

On the other hand, the first snippet using !person could call doSomething when person is undefined as well.

There can be subtle bugs when the code is not explicit. So, when possible (and linters can help with this) prefer to be explicit in your comparisons!

Named Conditions

Another rule that can help with complex conditionals is: Give each condition (or group of conditions) a name.

Consider the example below:

function processUser(user: User) {
  if (user.isAdmin && (user.username === "admin" || user.isActive)) {
    console.log("This is an admin user with a valid username or active account.");
  } else {
    console.log("This is a standard user.");
  }
}

An easy refactor could be just extracting the nested condition into its own variable, effectively giving it a name:

function processUser(user: User) {
  const hasProperSettings = user.username === "admin" || user.isActive;

  if (user.isAdmin && hasProperSettings) {
    console.log("This is an admin user with a valid username or active account.");
  } else {
    console.log("This is a standard user.");
  }
}

For more complex conditions you can consider extracting out several names.

Catch-all Conditions

This applies mostly to dynamically-typed languages, although it can also happen in statically-typed ones. It’s always a good idea to ensure you have a “catch-all” when using switch or if-elseif:

if (action === "create") {
  createUser();
} else if (action === "email") {
  emailUser();
}

When action is neither "create" nor "email", the code will silently just do nothing. And that could be very hard to debug.

Prefer this instead:

if (action === "create") {
  createUser();
} else if (action === "email") {
  emailUser();
} else {
  throw new Error(`Invalid action: "${action}"`)
}

Now we are actively reporting an error. If something goes wrong in the future for whatever reason, we know where to look. And that simple else at the end can save a lot of development time!

Conclusion

Those were a lot of rules! I hope they were useful, and can help you write code that’s easier to read and understand, and thus, easier to maintain and produce fewer bugs.

If you liked this list of common refactors, keep an eye out for future blog posts in this series!