AmouAI Hub/Courses/Programming Fundamentals/Day 22
Clean Code, Refactoring & Code Review
Readability as a feature. A deliberately ugly 200-line program, refactored live, plus the Git and review workflow around it.
By the end of today you can
- Name eight code smells on sight
- Apply the refactoring moves that fix each one
- Refactor in small steps with tests green the whole way
- Write a commit message a stranger could act on
- Open a pull request that is pleasant to review
- Give review feedback that is specific and kind
- Receive review feedback without taking it personally
▶Today's videos
Watch each video, then work the matching sections below. Watching alone will not do it.
git add -p · commit messages that say why · a pull request worth reviewing · giving and receiving feedback.1Code smells
Not bugs. Signals.
A smell is not a rule violation — it is something that is usually a symptom of a deeper problem. The value is in noticing, not in obeying.
| Smell | Looks like | Usually fixed by |
|---|---|---|
| Long function | Over ~30 lines, or needs section comments | Extract function |
| Long parameter list | 5+ parameters | Introduce an object |
| Duplicated code | The same shape twice | Extract function |
| Magic number | * 0.15 with no name | Extract constant |
| Deep nesting | Four levels of indent | Guard clauses, extract |
| Comment explaining what | # add 1 to total | Rename, then delete the comment |
| Feature envy | A method mostly using another object's data | Move the method |
| Primitive obsession | str for money, tuple for a point | Make a small class |
**A comment that explains what the code does is a function name in disguise.** Take the lines the comment covers, extract them into a function, and name the function what the comment said. The comment disappears and the code improves. This is Day 6's "name an idea" arriving as a mechanical procedure.
Count what changed: single-letter names got meanings, 0.15 and 0.08 became named constants, three levels of nesting collapsed into a filter, i[1] and i[2] became o.amount and o.is_active, and the rate decision moved into its own named function. The behaviour is identical.
2The refactoring moves
Small, named, reversible.
| Move | What you do | When |
|---|---|---|
| Extract function | Lift a block into a named function | The block needs a comment |
| Inline function | Replace a call with its body | The name adds nothing |
| Rename | Change a name to say what it means | You hesitated while reading it |
| Extract constant | Replace a literal with a named value | A magic number appears |
| Introduce parameter object | Group parameters into a class | Five+ parameters, or they travel together |
| Replace conditional with polymorphism | One class per branch | A type-checking if-chain |
| Guard clause | Return early on the awkward case | Deep nesting |
One move, run the tests, commit. Not five moves and a hopeful run. If the tests go red after a single small move, you know exactly what caused it and git checkout costs you thirty seconds. This is the same binary-search instinct from Day 5 and the same diff rule from Day 6.
Without a test suite you cannot tell restructuring from breaking. That is why Day 15 and Day 21 came first — and it is why "we will add tests later" so often means "we will stop being able to change this safely".
When not to refactor
- While adding a feature. Do one, commit, then do the other. A diff that mixes them cannot be reviewed.
- Code nobody touches. Ugly and stable beats clean and freshly broken.
- Without tests, unless you write characterisation tests first.
- On a deadline, unless the mess is what is slowing you down — which it often is.
3Git and review
The part of programming that is about other people.
git switch -c refactor/extract-tax-rate # a branch per change
git add -p # stage in pieces, review as you go
git commit -m "Extract rate_for() from total_tax"
git push -u origin refactor/extract-tax-rateCommit messages
The diff already shows what changed. Your message exists to record why — the thing that is otherwise lost forever. Same principle as comments on Day 5.
Reviewing
| Instead of | Write |
|---|---|
| "This is wrong." | "This returns None when items is empty — should it raise instead?" |
| "Bad naming." | "d took me a moment. discount_rate?" |
| "Why did you do this?" | "What was the reason for the second loop? I might be missing something." |
| "Add tests." | "Could we add a test for the empty case? That is the one I would break." |
As the reviewer: review the code, never the person. Say what you observed and what you would prefer, and ask rather than assert when you might be wrong. As the author: your code is not you. A reviewer finding a bug has done you a favour that would otherwise have been done by a user. The correct response to good feedback is "good catch", not a defence.
A pull request worth reviewing
- Small. 400 lines is a real review; 4,000 is a rubber stamp.
- One concern. Refactor or feature, never both.
- Green. Tests pass before you ask anyone to look.
- Described. What, why, and how you tested it.
- Self-reviewed. Read your own diff first — you will find something every time.
>_Python playground
A real Python interpreter running inside your browser. Nothing is installed, nothing is uploaded, nothing can break.
input(), comma separated →
✓Exercise set
Checked automatically the moment you submit. Work top to bottom — each one assumes the last. Your answers are saved in this browser.
Day 23 — Comparative Case Study + Capstone Kickoff
One application, built twice — and the capstone design document.