# Clean code

> Write code that reads top to bottom: intention-revealing names, small functions, guard clauses instead of arrow code, and comments that explain why.

Software Engineer roadmap · Code that lasts · 17 min read · https://levelcode.org/roadmaps/software-engineer/clean-code

Code is read far more often than it's written — by reviewers, by teammates fixing bugs at 2 a.m., and by you in six months with none of today's context. "Clean code" isn't a style preference; it's the property that lets someone change code safely without first reverse-engineering it. This lesson covers the habits with the biggest payoff: names, function size and shape, flattening nested logic with guard clauses, avoiding hidden inputs, and writing comments that add what the code can't say.

## Names that reveal intent

A good name tells the reader what a thing is for, so they don't need to read its implementation. Compare:

```go
// Before
func calc(d []Item, f bool) float64 {
	var t float64
	for _, x := range d {
		t += x.P * float64(x.Q)
	}
	if f {
		t *= 0.9
	}
	return t
}

// After
func OrderTotal(items []LineItem, isMember bool) float64 {
	var total float64
	for _, item := range items {
		total += item.UnitPrice * float64(item.Quantity)
	}
	if isMember {
		total *= 1 - memberDiscount
	}
	return total
}

const memberDiscount = 0.10
```

The logic is identical. The second version can be read aloud and understood. Some rules that hold across languages:

- **Name length should match scope.** A loop index used on one line can be `i`; a package-level function needs a full name. Go's convention of short local names (`r` for a reader, `ctx` for a context) works because their scope is a few lines.
- **Name by role, not by type.** `users` beats `userList`; `deadline` beats `timeObj`.
- **Booleans read as questions:** `isMember`, `hasExpired`, `canRetry`.
- **Functions are verbs or describe what they return:** `SendInvoice`, `OrderTotal`. In Go, getters drop the `Get` prefix: `user.Name()`, not `user.GetName()`.
- **Replace magic numbers with named constants.** `if retries > 3` hides a decision; `if retries > maxRetries` names it and gives it one place to change.
- **Use the domain's words.** If the business says "settlement", don't call it `processPayment2`. The [domain-driven design lesson](https://levelcode.org/roadmaps/software-engineer/domain-driven-design) builds on this.

## Small functions that do one thing

A function should do one thing, at one **level of abstraction**. When a function mixes high-level steps ("validate the order, charge the card, send a receipt") with low-level details (string trimming, SQL, HTTP headers), the reader has to switch altitude every few lines.

```go
func (s *CheckoutService) PlaceOrder(ctx context.Context, req PlaceOrderRequest) (Order, error) {
	if err := req.Validate(); err != nil {
		return Order{}, err
	}
	order, err := s.orders.Create(ctx, req.CustomerID, req.Items)
	if err != nil {
		return Order{}, fmt.Errorf("create order: %w", err)
	}
	if err := s.payments.Charge(ctx, order.ID, order.Total); err != nil {
		return Order{}, fmt.Errorf("charge order %s: %w", order.ID, err)
	}
	s.notifier.OrderPlaced(ctx, order)
	return order, nil
}
```

You can read this function like a paragraph. Each step's details live in a function named after what it does. "Small" isn't a line count — a 40-line function that builds one SQL query can be perfectly clear — but if you find yourself writing a comment to label a section of a function (`// now send the email`), that section usually wants to be a function with that name.

Keep parameter lists short. More than three or four parameters, or several of the same type next to each other (`func Move(x, y, dx, dy, speed int)`), invite mistakes; group them into a struct.

**Avoid boolean flag parameters.** `Render(doc, true)` tells the reader nothing at the call site, and the flag usually means the function does two things. Split it — `RenderDraft(doc)` and `RenderFinal(doc)` — or pass a named option.

## Guard clauses and the arrow anti-pattern

The **arrow anti-pattern** is code whose nested `if`s push the happy path further and further right, so the indentation forms an arrow:

```go
func (s *Service) Withdraw(ctx context.Context, accountID string, amount int64) error {
	account, err := s.repo.Find(ctx, accountID)
	if err == nil {
		if account.Active {
			if amount > 0 {
				if account.Balance >= amount {
					account.Balance -= amount
					return s.repo.Save(ctx, account)
				} else {
					return ErrInsufficientFunds
				}
			} else {
				return ErrInvalidAmount
			}
		} else {
			return ErrAccountClosed
		}
	} else {
		return err
	}
}
```

To understand any `else`, you have to match it with an `if` many lines above, and keep every enclosing condition in your head. The fix is the **guard clause**: check each failure condition first, return early, and let the happy path run down the left edge:

```go
func (s *Service) Withdraw(ctx context.Context, accountID string, amount int64) error {
	if amount <= 0 {
		return ErrInvalidAmount
	}
	account, err := s.repo.Find(ctx, accountID)
	if err != nil {
		return fmt.Errorf("find account %s: %w", accountID, err)
	}
	if !account.Active {
		return ErrAccountClosed
	}
	if account.Balance < amount {
		return ErrInsufficientFunds
	}
	account.Balance -= amount
	return s.repo.Save(ctx, account)
}
```

Same behaviour, no nesting, and each rule is one self-contained `if`. This is also simply idiomatic Go: `if err != nil { return ... }` is a guard clause, and Go style guides ask you to "indent error flow" and keep the normal path unindented. Moving the cheap `amount` check before the database call is a bonus: you no longer hit the database for requests you'll reject anyway.

Other ways to flatten arrows:

- **`continue` early in loops** instead of wrapping the whole body in an `if`.
- **Extract the inner block** into a function with its own guard clauses.
- **Replace a long `if/else if` chain on a type or status** with a `switch`, a lookup table, or polymorphism (see the [SOLID lesson](https://levelcode.org/roadmaps/software-engineer/solid)).

The [guard clauses concept note](https://levelcode.org/concepts/guard-clauses) has the same refactoring in TypeScript.

## No hidden inputs or surprises

A function is easy to reason about when everything it depends on comes in through its parameters and everything it changes goes out through its return values. Hidden inputs make code surprising:

- **Global mutable state.** A function that reads a package-level `var currentUser` behaves differently depending on who called what before it. Pass the value in.
- **Reading the clock or randomness directly.** `time.Now()` deep inside business logic makes it untestable at a fixed time. Accept a `now time.Time` or a clock interface.
- **Side effects behind innocent names.** `ValidateUser` that also writes to the database violates the reader's trust. A function named like a question should only answer it — the [design principles lesson](https://levelcode.org/roadmaps/software-engineer/design-principles) calls this command–query separation.

## Errors are part of the design

Handle errors at the level that can do something about them, and add context as they travel up. In Go, wrap with `%w` and say what you were doing: `fmt.Errorf("charge order %s: %w", id, err)`. The final message then reads like a stack of intentions: `place order: charge order 42: card declined`.

Don't swallow errors (`_ = s.repo.Save(ctx, a)`) unless you've decided — and written down in a comment — that failure truly doesn't matter. Don't log *and* return the same error; it gets logged once per layer and the log fills with duplicates. Either handle it (log, fall back, retry) or return it.

## Comments that earn their place

Good code makes most "what" comments unnecessary. Comments are for what the code *can't* say:

```go
// Bad: repeats the code
// increment retries
retries++

// Good: explains a decision a reader would otherwise "fix"
// The payment provider deduplicates on this key for 24 hours, so a retry
// within that window can never double-charge. Do not regenerate it per attempt.
idempotencyKey := order.ID + ":charge"
```

Write comments for: **why** a non-obvious choice was made, **warnings** about consequences, links between code that must change together, and doc comments on exported identifiers (Go's convention is a full sentence starting with the name: `// OrderTotal returns ...`). Delete commented-out code — version control remembers it. A `TODO` without a name or ticket is a wish, not a plan.

## Consistency beats preference

Format with the tool everyone uses — `gofmt` for Go, Prettier or Biome for TypeScript — and never argue about whitespace in review again. Beyond formatting, follow the conventions already in the codebase even when you'd choose differently: one consistent pattern is easier to read than a better pattern used in half the files. Change a convention deliberately, everywhere, or not at all.

## Practice

Refactor this TypeScript function. Keep its behaviour identical, remove the nesting with guard clauses, replace magic numbers with named constants, and give it a name that says what it does.

```ts
function check(u: { age?: number; country?: string; banned: boolean } | null): string {
  if (u) {
    if (!u.banned) {
      if (u.age !== undefined) {
        if (u.age >= 18) {
          if (u.country === 'ID' || u.country === 'MY') {
            return 'ok'
          } else {
            return 'region'
          }
        } else {
          return 'age'
        }
      } else {
        return 'age'
      }
    } else {
      return 'banned'
    }
  }
  return 'missing'
}
```

**Solution**

Each failure becomes a guard clause, in the original order of precedence. A union type documents every possible result.

```ts
const MINIMUM_AGE = 18
const SUPPORTED_COUNTRIES = new Set(['ID', 'MY'])

type Eligibility = 'ok' | 'missing' | 'banned' | 'age' | 'region'

type Applicant = { age?: number; country?: string; banned: boolean }

function checkEligibility(applicant: Applicant | null): Eligibility {
  if (!applicant) return 'missing'
  if (applicant.banned) return 'banned'
  if (applicant.age === undefined || applicant.age < MINIMUM_AGE) return 'age'
  if (!applicant.country || !SUPPORTED_COUNTRIES.has(applicant.country)) return 'region'
  return 'ok'
}
```

Check the edge cases match: a missing age returned `'age'` before and still does; a missing country returned `'region'` before (`undefined` matches neither `'ID'` nor `'MY'`) and still does. Before refactoring for real, you'd pin these behaviours with tests — the [refactoring lesson](https://levelcode.org/roadmaps/software-engineer/refactoring) covers that.

## Key takeaways

- Names should reveal intent; match name length to scope and use the domain's vocabulary.
- A function does one thing at one level of abstraction; split sections you'd label with a comment.
- Replace nested conditionals (the arrow anti-pattern) with guard clauses that return early, keeping the happy path unindented.
- Avoid hidden inputs — globals, the clock, side effects behind query-like names — so functions are predictable and testable.
- Add context to errors as they travel up; handle or return, don't do both.
- Comment the why, not the what; let formatters settle style and follow existing conventions.

## Quiz

1. What is the main problem with the arrow anti-pattern?

   A. It compiles to slower machine code
   B. Deeply nested conditions force the reader to track every enclosing condition and match distant else branches
   C. Go does not allow more than three levels of nesting
   D. It prevents the use of early returns

   **Answer: B.** The cost is cognitive. Guard clauses turn each condition into a self-contained check and keep the main path flat; performance is unaffected.

2. A function signature is Export(report Report, true). What is the best improvement?

   A. Rename the parameter to flag
   B. Add a comment at every call site explaining true
   C. Make the boolean a global setting
   D. Split it into two functions named for each behaviour, or pass a named option

   **Answer: D.** A bare boolean is unreadable at the call site and usually means the function does two things. Two well-named functions, or an options struct, make the intent explicit.

3. Which comment adds the most value?

   A. // Explain why the timeout is 25s: the upstream gateway cuts connections at 30s, and we need time to return a clean error
   B. // loop over the users
   C. // set x to 5
   D. // old implementation, kept just in case

   **Answer: A.** It records a constraint that isn't visible in the code and stops someone from 'tidying' the number. The others repeat the code or keep dead code that version control already stores.

4. Why is calling time.Now() deep inside a pricing function a problem?

   A. time.Now() is slow
   B. It is not safe for concurrent use
   C. It is a hidden input: results depend on when the code runs, so it is hard to test and reason about
   D. It always returns UTC

   **Answer: C.** Passing the time (or a clock) in as a parameter makes the function deterministic and lets tests check behaviour at specific moments, such as the end of a promotion.
