The Professor
Index Course index

Module 5 · Architecture and maintainable code

Lesson 28 — Refactoring without breaking anything

Test-backed refactoring: changing the inside without moving the contract.

Published
In this lesson
  1. Objectives
  2. 1. What is and what is not refactoring
  3. 2. The net: what each test layer covers
  4. 3. The guided refactor, step by step
  5. 4. Feigning: change NOTHING observable
  6. 5. The bridge to module 6: extracting the job
  7. Self-assessment

Stack: Django/DRF · Project: TicketFlow Status: Published — closing the architecture module Prerequisite: Lesson 27 — 12-factor configuration

Objectives

  1. Execute a real refactor backed by the suite: change the inside without moving the observable contract.
  2. Apply the "characterization + feigning" technique: pin the CURRENT behavior (bugs included) before touching anything.
  3. Prepare the ground for module 6: extract the reservation expiration job towards Celery without changing anything.

1. What is and what is not refactoring

Refactoring (Fowler): changing the internal structure without changing observable behavior. "Observable" here = the suite: 15/26's contract tests, HTTP responses, 25's outbox events, 45's logs with trace_id. If a refactor changes a response, it wasn't a refactor: it was a behavior change in disguise (and it needs its own task, its new test and its API version, 14).

The junior anti-pattern: "I'll fix the bug while I'm at it in the refactor". No: the bug gets fixed in a commit with a regression test BEFORE, and the refactor goes afterwards over already-pinned behavior. Mixing both = when something breaks you don't know whether it was the refactor or the fix.

2. The net: what each test layer covers

Before refactoring, audit your net (33 weaves it deliberately; here you only verify it exists):

Unit (33)               → pure logic: FakeClock, resumen(), state-machine rules
Integration (34)        → service + real Postgres: reservar() with transactions and locks
Contract (15/26)        → problem+json responses, the API's shape: the refactor doesn't touch them
E2E (35)                → the full purchase flow (slow, few, the final insurance)

If the code you are about to touch has no net, the technique is characterization (Feathers): write tests that PIN the current behavior, however ugly. assertEqual(resumen(r).total, 4250) — you don't even know whether 4250 is correct; you only know it was that. These tests are temporary pins: the refactor must not move them; afterwards, with the code clean, you rewrite them with the right expectations.

3. The guided refactor, step by step

Real case from the course: the reservation view (24's fat one) is already thin, but reservar() accumulated 180 lines after 25 (outbox, gateway, clock, locks). Goal: split it into a ReservacionService with clear dependencies without changing a SINGLE response. The mechanics:

bash
# 1. The whole suite green before touching ANYTHING (and time it: it's your duration baseline)
python manage.py test --parallel 4

# 2. The refactor's branch (even if the merge to trunk happens the same day)
git checkout -b refactor/reservacion-service

# 3. Small commit = test green after each step. If red: revert the step, no debugging
git commit -m "extract _validar_asientos() without behavior change"

This case's atomic steps: (1) extract _validar_asientos(event, seat_refs, repos) — pure logic, new unit test; (2) extract _cobrar(gateway, amount, intent) — the infra boundary, 24 already defined the Protocol; (3) move the transaction and the outbox into ReservacionService.reservar(); (4) the view goes from calling the loose function to calling the service — the deprecation way: the old function delegates to the service for a sprint and dies with its deprecation test. Each step: suite green → commit. If a step requires "fixing 12 tests", the step was two steps.

4. Feigning: change NOTHING observable

The enemies of the honest refactor: the "details" that are actually contract. Checklist of untouchables: status codes and bodies (26), JSON field names (15), public IDs (UUID, 13) — a refactor that renumbers visible PKs is an API change. Also: config defaults (27), error messages the front parses (if the front does if err.type === "seat-unavailable", that string is contract), and side effects: emails, webhooks, outbox. The classic trap: reordering the outbox and publishing BEFORE the commit — 25's tests (event after transaction) catch it; if they didn't exist, you would have broken the guarantee without knowing.

Signal of a healthy refactor: the git diff of the tests in the PR is ~zero (characterization tests untouched); the code diff is large; the output diff (response snapshots in tests) is ZERO.

5. The bridge to module 6: extracting the job

The course's closing proposes: reservation expiration today is a manual cron (manage.py expirar_reservas running in a terminal tab, 00's embarrassment). The refactor opening module 6: extract the LOGIC (expirar_reservas(clock) in the service, pure, tested with FakeClock) from the TRIGGER (the manage.py command, later a Celery worker from 29). The general rule: extract the logic from the mechanism first — the refactor leaves the logic in the service with its stable signature, and module 6 only changes who calls it. This is also the pattern of every infrastructure migration: stable logic, replaceable transport.

Self-assessment

  1. What distinguishes a refactor from a disguised behavior change, and which suite defines "observable" in this course?
  2. What is a characterization test and why are "ugly" assertions (4250) allowed, to be rewritten later?
  3. Why is "fixing the bug in passing" in the refactor's PR the costliest decision?
  4. List 5 "observable" things a refactor must NOT change even if the code diff is legitimate.
  5. In the expiration job extraction: what gets extracted first (logic or transport), and what does it leave ready for 29?

Continue with the exercises. The solutions only after trying it yourself.

^ top