Contact
Evil Genius SocietyHomeLearnForumLeaderboardSocialsMessages
← Back to module

Lesson 5 of 6

Your capstone works. That's not the same as being finished.

Code review is reading code deliberately, looking for what's wrong with it — before someone else finds out the hard way. On a team, someone else does it for you. Alone, you do it for yourself, and it's the single fastest way to improve.

Do it on a different day from writing. You can't see your own code clearly an hour after writing it.

The checklist

Does it actually work?

  • [ ] The normal case produces the right answer — checked against something you computed by hand
  • [ ] Empty input: empty list, empty file, empty string, zero
  • [ ] One item, and a lot of items
  • [ ] Bad input: wrong type, missing key, malformed file, negative number
  • [ ] Running it twice in a row does the right thing
  • [ ] Nothing is left behind — temp files cleaned up, files closed

Can someone read it?

  • [ ] Every name says what the thing is (total_spent, not t)
  • [ ] Functions do one thing, and their name says which
  • [ ] Nothing is nested more than three levels deep
  • [ ] No function longer than about 30 lines
  • [ ] No number appears with no explanation — name it (MAX_RETRIES = 3)
  • [ ] Comments say why; the code says what
  • [ ] Public functions and classes have docstrings
  • [ ] PEP 8 clean (lesson 9.8) — let ruff or black decide

Does it fail well?

  • [ ] Every except names the exception it's catching — never bare except:
  • [ ] No except: pass that hides a real problem
  • [ ] Error messages tell the user what to do, not what Python thinks
  • [ ] The user never sees a raw traceback
  • [ ] Files, connections, and locks are opened with with
  • [ ] Data is validated at the boundary, where it comes in

Is it safe?

  • [ ] No passwords, API keys, or tokens in the source — environment variables only (lesson 8.3)
  • [ ] Nothing secret in the git history either
  • [ ] No eval() or exec() on anything a user typed
  • [ ] Nothing overwrites or deletes a file without checking first
  • [ ] Destructive operations have a dry-run mode (lesson 10.3)

Can you change it in six months?

  • [ ] Tests for the core logic, and for every bug you've fixed (lesson 9.9)
  • [ ] A README.md: what it is, how to install it, how to run it
  • [ ] A requirements.txt (lesson 7.6)
  • [ ] A .gitignore — __pycache__/, .venv/, *.json data files, .env
  • [ ] Configuration is separate from logic — no hardcoded paths
  • [ ] Logic is separate from interface: the class computes, the CLI prints

Review this

Here's a function that works, reviewed and rewritten. Same answer, different code.

Everything the checklist caught in six lines:

| Problem | Fix | | --- | --- | | p, d, f, x, y, t | Names that say what they hold | | File never closed | with open(...) | | No encoding | encoding="utf-8" — the default varies by platform | | for i in range(len(y)) | Loop over the items | | t = t + ... accumulator | sum(...) with a generator | | Prints and returns | Return the value; let the caller print | | "total is " + str(t) | f-string | | No docstring | One line saying what it returns |

That's not style pedantry. The version with with open can't leak a file handle. The version that only returns can be tested and reused. The sum version has nowhere for an off-by-one to hide.

Three traps a checklist catches

These are real bugs, they don't raise errors, and every Python programmer has shipped at least one.

Mutable default argument. basket=[] is evaluated once, when the function is defined — so every call shares one list. Look at that output: the basket keeps filling up across separate calls. Default to None and build the real value inside.

Bare except. It catches everything, including the NameError from your typo three lines down. You'll spend an hour on a bug the interpreter would have handed you instantly. Name your exceptions, and return what you rejected instead of dropping it silently.

Float equality. 0.1 + 0.2 == 0.3 is False. Compare with a tolerance, or use integer cents (lesson 10.2).

Let tools do the boring half

pip install ruff pytest
ruff format .        # rewrite to a consistent style
ruff check .         # unused imports, shadowed builtins, bare except, and more
pytest               # run the tests

ruff check catches a real fraction of the checklist above, in under a second, without opinions. Add mypy if you want type checking, and pytest --cov to see which lines your tests never touch.

Run these before every commit. What's left for your human review is the part tools can't do: is this the right thing to build, and will the next person understand it?

The README

Nobody can use a project they can't run. Yours needs, at minimum:

# Expense Tracker

A command-line tool for recording expenses and reporting on them by month.

## Install

    python -m venv .venv
    source .venv/bin/activate
    pip install -r requirements.txt

## Use

    python tracker.py add --amount 12.50 --category food
    python tracker.py report 2026-03

Data is stored in `expenses.json` in the working directory.

## Run the tests

    python -m unittest

Write it for yourself in six months, because that's who'll read it.

Reviewing other people's code

Once you're working with other people, the rules change:

  • Review the code, never the person. "This loop could be a comprehension," not "you always do this."
  • Ask, don't decree. "What happens if items is empty?" invites a fix. "This is broken" invites a fight.
  • Say what's good. A review that's only criticism is exhausting to receive, and you learn from good code too.
  • Separate must-fix from nice-to-have. Prefix the optional ones: "nit: extra blank line."
  • Approve when it's better than what's there. Perfect is not the bar; better is.

And when you're on the receiving end: it's not about you. Every review comment is someone spending their time making your code better.

Checkpoint

  • Review on a different day from writing.
  • Five questions: does it work, can it be read, does it fail well, is it safe, can it be changed?
  • Watch for mutable defaults, bare except, and float equality.
  • ruff and pytest do the mechanical half — run them before every commit.
  • A README and a .gitignore are part of "done".
  • Review code, not people; ask questions; say what's good.

Questions about this lesson

Log in to ask a question.