Refactoring — code smells, common refactorings (extract method, rename, etc.): Exercises

Before we start, three tiny definitions we will count with in later problems:

The loudest smell is Magic Number: 1.0825 is an unexplained literal. It hides a meaning (an 8.25% tax multiplier) inside a bare number. A close second is poor Naming (f, t, i), but the question asked for the single loudest — the magic number is the one that will silently break if the tax rate changes and nobody knows where to look. Fix: TAX_RATE = 0.0825. See Naming Conventions.

L1·Q2 — Refactoring or not?

A developer changes a function so it now returns -1 on empty input instead of 0. Is this a refactoring? Yes/No + one-line reason.

Recall Solution — L1·Q2

No. Refactoring is behaviour-preserving. The observable output for the empty-input case changed from 0 to -1, so the external behaviour changed. That is a bug fix or a feature change, not a refactoring.

L1·Q3 — Match the fix

Match each smell to its primary refactoring: (a) Long Method (b) Duplicated Code (c) Long Parameter List (d) Data Clump

Recall Solution — L1·Q3

(a) Long Method → Extract Method
(b) Duplicated Code → Extract Method + call it in both places (removing duplication is the goal; extracting the shared idea is the move). See DRY Principle.
(c) Long Parameter List → Introduce Parameter Object / Extract Class
(d) Data Clump → Extract Class (bundle the clumped variables into one object)

Level 2 — Application

L2·Q1 — Count the cyclomatic complexity

For the function below, compute $M = D + 1$ where $D$ = number of decision points.

def grade(score):
    if score >= 90:        # 1
        return "A"
    elif score >= 80:      # 2
        return "B"
    elif score >= 70:      # 3
        return "C"
    else:
        return "F"

Recall Solution — L2·Q1

WHAT: count decision points. Each if/elif is one branch; else is not a decision point (it's the default path, no condition). So $D = 3$ . WHY $M = D + 1$ : the "+1" is the single path that flows straight through with no branch taken; every decision adds one more independent route. $M = D + 1 = 3 + 1 = 4$ Meaning: you need at least 4 test cases (one score in each band: e.g. 95, 85, 75, 50) to exercise every path.

L2·Q2 — Apply Extract Method

Refactor this by extracting the eligibility check into a well-named function. Behaviour must stay identical.

def checkout(user, cart):
    if (user.age >= 18) and (user.country in ALLOWED) and user.verified:
        return process(cart)
    return deny()

Recall Solution — L2·Q2

Extract the boolean into a named method whose name states the concept:

def can_purchase(user):
    is_adult  = user.age >= 18
    region_ok = user.country in ALLOWED
    return is_adult and region_ok and user.verified
 
def checkout(user, cart):
    if can_purchase(user):
        return process(cart)
    return deny()

Why this step: the tangled boolean is a self-contained idea ("is this user allowed to buy?"). Naming it makes checkout readable at a glance and makes the rule independently testable. Same truth value in, same branch taken out ⇒ behaviour preserved. This also fixes the Single Responsibility pressure on checkout. See Single Responsibility Principle and Clean Code.

L2·Q3 — Rule of Three

The same 3-line validation block appears in create_user, update_user, and import_user. According to the Rule of Three, should you refactor? How many copies trigger the rule?

Recall Solution — L2·Q3

Yes, refactor now. The Rule of Three: copy once (2 total occurrences) = tolerate; the third occurrence (3 total) is the trigger. Here there are 3 copies ⇒ extract the block into one shared validate_user(...) and call it from all three. This removes Duplicated Code and prevents fix-in-one-forget-the-other bugs. See DRY Principle.

Level 3 — Analysis

L3·Q1 — Compound-condition complexity

Boolean operators and/or are also decision points (each can short-circuit). Compute $M$ for:

def route(pkt):
    if pkt.valid and pkt.size < 1500 and (pkt.priority or pkt.urgent):
        return fast()
    return slow()

Recall Solution — L3·Q1

WHAT: count every decision point. Decision points: the if itself, and each and/or. Points: if (1), and (2), and (3), or (4). So $D = 4$ . WHY count and/or: each can short-circuit — it introduces an independent true/false path through the code, exactly like an if. $M = D + 1 = 4 + 1 = 5$ Analysis: $M = 5$ from one line is a warning — the compound condition packs 5 paths into a single if. Extracting explaining variables (as in L2·Q2) does not reduce $M$ , but it makes those 5 paths readable, which is often the real goal.

L3·Q2 — Which refactoring, and why not the other?

A method total_price(x1, y1, x2, y2, x3, y3) keeps receiving coordinates in triples. Two candidate fixes: (A) Extract Method, (B) Extract Class (Point). Which is correct and why is the other wrong here?

Recall Solution — L3·Q2

Correct: (B) Extract Class. The six arguments are secretly three Point objects — a classic Data Clump and Long Parameter List. Bundling (x, y) into Point(x, y) turns the signature into total_price(p1, p2, p3), killing both smells at once. Why (A) is wrong here: Extract Method pulls a fragment of logic into its own function — it addresses Long Method, not a parameter shape. The problem isn't that the body is long; it's that the data travels in clumps. Extracting a method would leave all six parameters in place. Right tool for the right smell.

L3·Q3 — Read the safety invariant

Recall the invariant: a valid step is a transformation $f$ where $T (code) = PASS ⟹ T (f (code)) = PASS and B (f (code)) = B (code)$ Here $T$ = the test suite, $B$ = observable behaviour. A refactor keeps all tests green but a hidden edge case (untested) now returns the wrong value. Did the refactor satisfy the invariant?

Recall Solution — L3·Q3

It satisfied the test condition but violated the behaviour condition. Tests $T$ stayed green, yet $B (f (code)) \neq = B (code)$ on the untested edge case — real behaviour changed. This is exactly the invariant's warning: tests are only a proxy for $B$ . The weaker $T$ approximates $B$ , the more silent behaviour changes slip through. Lesson: strengthen $T$ (add the edge-case test) before trusting a refactor. See Unit Testing.

Level 4 — Synthesis

L4·Q1 — Full refactor with a complexity target

Refactor the function below. Requirements: (1) behaviour identical, (2) no magic numbers, (3) reduce the top-level function's cyclomatic complexity to $M = 1$ by extracting the decision logic. State the before and after $M$ of describe.

def describe(order):
    s = 0
    for i in order.items:
        s += i.price * i.qty
    if s > 100:
        s = s - s * 0.10
    s = s * 1.0825
    print("Total: " + str(round(s, 2)))

Recall Solution — L4·Q1

Before — complexity of describe: decision points are the for (1) and the if (2), so $D = 2$ and $M_{before} = D + 1 = 2 + 1 = 3.$ Refactor — extract each idea, name the constants:

TAX_RATE       = 0.0825
DISCOUNT_RATE  = 0.10
DISCOUNT_FLOOR = 100
 
def subtotal(order):                       # M = 2 (one loop)
    return sum(i.price * i.qty for i in order.items)
 
def apply_discount(amount):                # M = 2 (one if)
    if amount > DISCOUNT_FLOOR:
        return amount * (1 - DISCOUNT_RATE)
    return amount
 
def compute_total(order):                  # M = 1 (no branches)
    return apply_discount(subtotal(order)) * (1 + TAX_RATE)
 
def describe(order):                        # M = 1 (no branches)
    print("Total: " + str(round(compute_total(order), 2)))

After — complexity of describe: it now has zero decision points, so $M_{after} = 0 + 1 = 1.$ Behaviour check: the arithmetic is unchanged — subtotal, then conditional 10% discount, then 8.25% tax, then print rounded to 2 dp. Same inputs → same printed string. We only relocated the branches into named, testable helpers. This applies Extract Method + Replace Magic Number with Constant together.

L4·Q2 — Numeric behaviour-preservation proof

Using the L4·Q1 constants, an order has items [($40, 2), ($30, 1)] (price, qty). Compute the printed total for both the original and refactored code and confirm they match.

Recall Solution — L4·Q2

Subtotal: $40 \times 2 + 30 \times 1 = 80 + 30 = 110$ . Discount: $110 > 100$ , so apply 10%: $110 \times (1 - 0.10) = 110 \times 0.90 = 99$ . Tax: $99 \times (1 + 0.0825) = 99 \times 1.0825 = 107.1675$ . Rounded to 2 dp: $107.17$ . Both versions run the same three operations in the same order ⇒ both print Total: 107.17. Behaviour preserved. ✓

Level 5 — Mastery

L5·Q1 — Design the minimal test set from complexity

Consider the apply_discount helper from L4·Q1:

def apply_discount(amount):
    if amount > DISCOUNT_FLOOR:
        return amount * (1 - DISCOUNT_RATE)
    return amount

$M = 2$ . Design the minimum set of test cases that covers every path, and include the boundary value. State inputs and expected outputs.

Recall Solution — L5·Q1

$M = 2$ means 2 independent paths ⇒ minimum 2 tests to touch both branches, but a careful engineer adds the boundary because > vs >= bugs live exactly there.

Input `amount`	Path taken	Expected output	Reason
$150$	`if` true	$150 \times 0.9 = 135$	above floor, discount applies
$50$	`if` false	$50$	below floor, no discount
$100$ (boundary)	`if` false ( $100 > 100$ is false)	$100$	the trap value — exactly at floor gets no discount

The boundary test $100$ pins down that we use strict >, not >=. That single test is what catches a silent flip during future refactoring. See Unit Testing.

L5·Q2 — Diagnose the deepest smell and prescribe the sequence

A Report class has 40 methods; 12 of them mostly read and mutate the fields of a separate Customer object rather than Report's own fields. Name all smells present, name the deepest one, and give the ordered refactoring sequence to fix it safely.

Recall Solution — L5·Q2

Smells present:

Large Class / God Object — 40 methods in one class hoards responsibilities.
Feature Envy — the 12 methods are "envious" of Customer's data; they belong with that data.

Deepest one: Feature Envy is the root here — those 12 methods being in the wrong place is part of why Report grew into a God Object. Fixing envy shrinks the class as a side effect.

Ordered safe sequence:

Cover with tests — ensure the 12 methods' behaviour is captured. (Golden rule: never refactor without tests.)
Move Method — relocate each envious method onto Customer, one at a time, running tests after each move (green keep / red revert).
Rename any method whose name assumed a Report context but now lives on Customer.
After the moves, re-evaluate Report: the God Object smell should have measurably eased (fewer methods, tighter responsibility).
Commit each move separately.

This respects Single Responsibility Principle: data and the behaviour that operates on it live together.

L5·Q3 — Steel-man the anti-refactor argument

A senior says: "Don't over-extract. Ten one-line helpers to avoid one 10-line method is worse." Is this ever correct? Give the principle that decides when extraction helps vs hurts.

Recall Solution — L5·Q3

Yes, sometimes correct. Over-extraction creates Shotgun-style indirection: to follow one idea you bounce through many files, and the cost of reading rises even as line-counts fall.

The deciding principle: extract when the fragment names a concept — a coherent idea worth a name (can_purchase, apply_discount). Do not extract merely to cut line count. A helper that is only ever called once and whose name adds no meaning beyond its body is pure indirection. Rule of thumb: if you can't give it a name clearer than its code, don't extract it.

This is the balance point between Long Method (too much in one place) and needless indirection (too little in too many places). Good refactoring optimizes understanding, not metrics.

Exercises — Refactoring — code smells, common refactorings (extract method, rename, etc.)

Level 1 — Recognition

L1·Q1 — Name the smell