The "bad" code

Here's a function that calculates the final price of an order. It applies a discount based on the user type (1 for regular, 2 for premium) and adds tax. It works, but it's hard to read and maintain.

function calculatePrice(items: any, userType: number) {
  let total = 0;
  for (let i = 0; i < items.length; i++) {
    // Calculate subtotal for each item
    total += items[i].price * items[i].qty;
  }

  // Apply discount
  if (userType === 1) { // Regular user
    total = total * 0.95; // 5% discount
  } else if (userType === 2) { // Premium user
    total = total * 0.85; // 15% discount
  }

  // Add tax
  total = total * 1.10; // 10% tax

  // Return the final price
  return total;
}

// Define the order items
const orderItems = [{ price: 10, qty: 2 }, { price: 25, qty: 1 }];
// Calculate the final price for a premium user
const finalPrice = calculatePrice(orderItems, 2); // Premium user
console.log(`Final price: ${finalPrice}`);

There are several issues with this code:

• Excessive comments that explain what the code does.
• Unclear variable names (items, userType, total).
• Magic numbers (1, 2, 0.95, 0.85, 1.10).
• Multiple responsibilities (calculating subtotal, applying discount, adding tax).
• Weak type safety.

Remove comment noise

I quite commonly see codebases that are filled with comments that explain what the code does. This is merely a band-aid for unclear code. Instead, strive to write code that is self-explanatory. Comments add a lot of noise, require maintenance, and can become outdated.

So when should you use comments?

• When you need to explain why you did something, such as a trade-off, architecture decision, or a business rule.
• When you need to add a TODO or FIXME to remind yourself or others to come back to something.

See the difference for yourself, compare the noisy "bad code" above to the version below, which is much less noisy without explanatory comments:

function calculatePrice(items: any, userType: number) {
  let total = 0;
  for (let i = 0; i < items.length; i++) {
    total += items[i].price * items[i].qty;
  }

  if (userType === 1) {
    total = total * 0.95;
  } else if (userType === 2) {
    total = total * 0.85;
  }

  total = total * 1.10;

  return total;
}

const orderItems = [{ price: 10, qty: 2 }, { price: 25, qty: 1 }];
const finalPrice = calculatePrice(orderItems, 2);
console.log(`Final price: ${finalPrice}`);

Add meaningful names and types

The beauty of this principle is that it's so simple that it's easy to overlook. But don't sleep on it, it makes a huge difference. It's about giving names to variables, parameters, and functions that make sense and are descriptive.

So how do you write good names?

• Reveal intent: The name should clearly state what the variable represents or does.
• Be specific & descriptive: Avoid vague terms like data or temp. Use names like orderItems instead of just items.
• Use relevant terms: Use terms that are specific to the domain or subject matter, such as discountRate or customerAddress.
• Follow conventions: Prefix boolean variables with is or has e.g. isLoading, hasPermission.
• Prioritize clarity: Choose longer, descriptive names instead of short, unclear abbreviations.
• Distinguish meaningfully: Make sure names clearly differentiate variables with similar roles (e.g., subtotal, discountedTotal, finalTotal).
• Be searchable: Avoid single letters or cryptic names that are hard to find in the codebase like i, x, y.
• Avoid misleading names: Make sure variable names accurately reflect their content or purpose to prevent confusion.

type OrderItem = {
  price: number;
  quantity: number;
};

type UserType = 'regular' | 'premium';

function calculateOrderTotal(orderItems: OrderItem[], userType: UserType): number {
  let subtotal = 0;
  for (let i = 0; i < orderItems.length; i++) {
    subtotal += orderItems[i].price * orderItems[i].quantity;
  }

  if (userType === 'regular') {
    subtotal = subtotal * 0.95;
  } else if (userType === 'premium') {
    subtotal = subtotal * 0.85;
  }

  const finalTotal = subtotal * 1.10;

  return finalTotal;
}

const items: OrderItem[] = [{ price: 10, quantity: 2 }, { price: 25, quantity: 1 }];
const price = calculateOrderTotal(items, 'premium');
console.log(`Final price: ${price}`);

We didn't make any changes to the logic of the code, yet it is now much easier to understand and reads like english.

Here are the improvements we made:

• Variable names: The improved variable names make the code self-documenting.
• Order item type: We added a type for the order item. Only use the any type as a last resort. What is the point of using typescript if you're not going to use types?
• User type: We changed the userType parameter from a number to a string literal. Having to mentally map a number to a user type is error prone and hard to understand. You constantly have to remind yourself that 1 is regular and 2 is premium. Now when you pass arguments to the function, you get the added benefit of autocompletion.

Eliminate magic numbers

Magic numbers are unnamed numerical constants in code that lack context or explanation. For example, seeing 0.95 in a calculation without any indication of its purpose can be confusing.

Trying to make sense of magic numbers and what they represent adds extra cognitive load and can send you down a rabbit hole of trying to figure out what a magic number represents.

Also, if you use the magic number in multiple places, you have to update it in every place you use it.

Fortunately, this problem has an easy fix: use named constants. Using named constants instead of hardcoded values for things like discount rates and tax multipliers clarifies the code's purpose and makes future changes easier.

The convention for naming constants in javascript/typescript codebases is to use all caps delimited by underscores.

type OrderItem = {
  price: number;
  quantity: number;
};

type UserType = 'regular' | 'premium';

const REGULAR_USER_DISCOUNT_RATE = 0.95 as const;
const PREMIUM_USER_DISCOUNT_RATE = 0.85 as const;
const TAX_RATE_MULTIPLIER = 1.10 as const;

function calculateOrderTotal(orderItems: OrderItem[], userType: UserType): number {
  let subtotal = 0;
  for (let i = 0; i < orderItems.length; i++) {
    subtotal += orderItems[i].price * orderItems[i].quantity;
  }

  if (userType === 'regular') {
    subtotal = subtotal * REGULAR_USER_DISCOUNT_RATE;
  } else if (userType === 'premium') {
    subtotal = subtotal * PREMIUM_USER_DISCOUNT_RATE;
  }

  const finalTotal = subtotal * TAX_RATE_MULTIPLIER;

  return finalTotal;
}

const items: OrderItem[] = [{ price: 10, quantity: 2 }, { price: 25, quantity: 1 }];
const price = calculateOrderTotal(items, 'premium');
console.log(`Final price: ${price}`);

Apply single responsibility principle (SRP)

This is hands down the most important principle of clean code.

Ok, so what does this actually mean? It's basically that a file, class, or function should do one thing and one thing only. It's about dividing responsibilities and making sure each function is focused on a single task.

For example:

• A function that calculates the subtotal of an order.
• A function that applies a discount to the subtotal.
• A function that applies tax to the subtotal.

You wouldn't want to have a function that does all of these things (unless it's an orchestrator function).

Breaking the main function into smaller, focused functions (calculating subtotal, applying discount, applying tax) makes each part easier to understand, test, and reuse.

type OrderItem = {
  price: number;
  quantity: number;
};

type UserType = 'regular' | 'premium';

const REGULAR_USER_DISCOUNT_RATE = 0.95 as const;
const PREMIUM_USER_DISCOUNT_RATE = 0.85 as const;
const TAX_RATE_MULTIPLIER = 1.10 as const;

const DISCOUNT_RATES: Record<UserType, (amount: number) => number> = {
  regular: (amount) => amount * REGULAR_USER_DISCOUNT_RATE,
  premium: (amount) => amount * PREMIUM_USER_DISCOUNT_RATE,
} as const;

function calculateSubtotal(orderItems: OrderItem[]): number {
  return orderItems.reduce((acc, item) => acc + item.price * item.quantity, 0);
}

function applyDiscount(amount: number, userType: UserType): number {
  return DISCOUNT_RATES[userType](amount);
}

function applyTax(amount: number): number {
  return amount * TAX_RATE_MULTIPLIER;
}

function calculateOrderTotal(orderItems: OrderItem[], userType: UserType): number {
  const subtotal = calculateSubtotal(orderItems);
  const discountedTotal = applyDiscount(subtotal, userType);
  const finalTotal = applyTax(discountedTotal);
  return finalTotal;
}

const items: OrderItem[] = [{ price: 10, quantity: 2 }, { price: 25, quantity: 1 }];
const price = calculateOrderTotal(items, 'premium');
console.log(`Final price: ${price}`);

Just take a moment to appreciate how much cleaner this code is compared to the previous version. Now we have small focused functions that do exactly what the function name implies.

This is a good point to stop refactoring as it's now in good shape.