Every codebase has one class that does everything. Here is how to recognise the smells, split it in small steps without changing a single total, and stop before the cure is worse than the disease.
Every team has one. It started as a 30-line CheckoutService, and then it learned to apply coupons, add tax, charge a card, handle cash on delivery and email a receipt. Nobody calls it a design. It is "the checkout", it is 400 lines, and the sentence you hear about it is don't touch that. The one in this lesson is 87 lines in Python with its demo, so it fits on a page, but it has the same shape.
Most lessons in this part start from a pattern and show where it fits. This one starts from the code. An anti-pattern is a common solution that looks fine on day one and makes every later change harder; a code smell is the hint that one is there. Refactoring to a pattern means moving to a pattern in steps that each leave the behaviour exactly as it was. The marks in a machine-coding round go to the candidate who can name the smell in the first version, fix it in small steps, and say why they stopped.
You have met these smells before: the parking lot lists a ParkingLot god class under its common mistakes, and other case studies list the rest. This lesson takes one messy class, names its smells, points at the lesson that cures each (single responsibility, Strategy, Singleton, dependency injection), refactors it in six steps in four languages, and then shows what going too far looks like.
Refactoring is moving furniture with a photo of the room: after every move you compare, so a missing chair is always the last move.
The first version: a checkout that does everything#
Here is the class as most teams have it. It reads two global objects, parses coupon codes from a string, compares payment names with if, and sends the receipt itself:
# checkout_v1.py: one class that prices, charges, emails and reads two globals
class Config:
_instance = None
@classmethod
def get(cls):
if cls._instance is None:
cls._instance = Config()
return cls._instance
def __init__(self):
self.tax_percent = 5
self.free_delivery_from = 500
self.delivery_fee = 40
class Mailer:
_instance = None
@classmethod
def get(cls):
if cls._instance is None:
cls._instance = Mailer()
return cls._instance
def __init__(self):
self.sent = []
def send(self, text):
self.sent.append(text) # a real mailer would call an email service here
class CheckoutService:
def place(self, items, coupon, payment):
cfg = Config.get()
subtotal = 0
for name, price, qty in items:
subtotal += price * qty
if coupon == "":
discount = 0
elif coupon.startswith("FLAT"):
discount = min(int(coupon[4:]), subtotal)
elif coupon.startswith("OFF"):
discount = subtotal * int(coupon[3:]) // 100
else:
raise ValueError("unknown coupon: " + coupon)
taxable = subtotal - discount
delivery = 0 if taxable >= cfg.free_delivery_from else cfg.delivery_fee
tax = taxable * cfg.tax_percent // 100
before_fee = taxable + delivery + tax
if payment == "card":
fee = before_fee * 2 // 100
elif payment == "upi":
fee = 0
elif payment == "cod":
fee = 20
else:
raise ValueError("unknown payment: " + payment)
total = before_fee + fee
status = "pending" if payment == "cod" else "paid"
Mailer.get().send("receipt: " + str(total))
return {"total": total, "status": status}
if __name__ == "__main__":
desk = [("notebook", 120, 2), ("pen", 25, 4)]
bag = [("bag", 800, 1)]
both = [("bag", 800, 1), ("pen", 25, 4)]
service = CheckoutService()
orders = [
("A", desk, "", "upi"),
("B", desk, "FLAT50", "card"),
("C", bag, "OFF10", "cod"),
("D", both, "FLAT1000", "upi"),
]
for label, items, coupon, payment in orders:
r = service.place(items, coupon, payment)
print(f"{label}: {r['total']} {r['status']}")
for items, coupon, payment in [(desk, "GOLD", "upi"), (desk, "", "cheque")]:
try:
service.place(items, coupon, payment)
except ValueError as e:
print("rejected:", e)
print("receipts sent:", len(Mailer.get().sent))
Config.get().tax_percent = 10 # one test changes a setting...
r = service.place(desk, "", "upi") # ...and every later caller sees it
print("after one test sets tax to 10, order A costs", r["total"])// CheckoutV1.java: one class that prices, charges, emails and reads two globals
import java.util.ArrayList;
import java.util.List;
class CheckoutV1 {
record Line(String name, int price, int qty) {}
static class Config {
private static Config instance;
static Config get() {
if (instance == null) instance = new Config();
return instance;
}
int taxPercent = 5, freeDeliveryFrom = 500, deliveryFee = 40;
}
static class Mailer {
private static Mailer instance;
static Mailer get() {
if (instance == null) instance = new Mailer();
return instance;
}
final List<String> sent = new ArrayList<>();
void send(String text) { sent.add(text); } // a real mailer would call an email service here
}
static class CheckoutService {
String place(List<Line> items, String coupon, String payment) {
Config cfg = Config.get();
int subtotal = 0;
for (Line line : items) subtotal += line.price() * line.qty();
int discount;
if (coupon.equals("")) {
discount = 0;
} else if (coupon.startsWith("FLAT")) {
discount = Math.min(Integer.parseInt(coupon.substring(4)), subtotal);
} else if (coupon.startsWith("OFF")) {
discount = subtotal * Integer.parseInt(coupon.substring(3)) / 100;
} else {
throw new IllegalArgumentException("unknown coupon: " + coupon);
}
int taxable = subtotal - discount;
int delivery = taxable >= cfg.freeDeliveryFrom ? 0 : cfg.deliveryFee;
int tax = taxable * cfg.taxPercent / 100;
int beforeFee = taxable + delivery + tax;
int fee;
if (payment.equals("card")) {
fee = beforeFee * 2 / 100;
} else if (payment.equals("upi")) {
fee = 0;
} else if (payment.equals("cod")) {
fee = 20;
} else {
throw new IllegalArgumentException("unknown payment: " + payment);
}
int total = beforeFee + fee;
String status = payment.equals("cod") ? "pending" : "paid";
Mailer.get().send("receipt: " + total);
return total + " " + status;
}
}
public static void main(String[] args) {
List<Line> desk = List.of(new Line("notebook", 120, 2), new Line("pen", 25, 4));
List<Line> bag = List.of(new Line("bag", 800, 1));
List<Line> both = List.of(new Line("bag", 800, 1), new Line("pen", 25, 4));
CheckoutService service = new CheckoutService();
System.out.println("A: " + service.place(desk, "", "upi"));
System.out.println("B: " + service.place(desk, "FLAT50", "card"));
System.out.println("C: " + service.place(bag, "OFF10", "cod"));
System.out.println("D: " + service.place(both, "FLAT1000", "upi"));
try {
service.place(desk, "GOLD", "upi");
} catch (IllegalArgumentException e) {
System.out.println("rejected: " + e.getMessage());
}
try {
service.place(desk, "", "cheque");
} catch (IllegalArgumentException e) {
System.out.println("rejected: " + e.getMessage());
}
System.out.println("receipts sent: " + Mailer.get().sent.size());
Config.get().taxPercent = 10; // one test changes a setting...
String again = service.place(desk, "", "upi"); // ...and every later caller sees it
System.out.println("after one test sets tax to 10, order A costs " + again.split(" ")[0]);
}
}// checkout_v1.cpp: one class that prices, charges, emails and reads two globals
#include <iostream>
#include <stdexcept>
#include <string>
#include <vector>
struct Line {
std::string name;
int price;
int qty;
};
struct Config {
int taxPercent = 5, freeDeliveryFrom = 500, deliveryFee = 40;
static Config& get() {
static Config instance;
return instance;
}
};
struct Mailer {
std::vector<std::string> sent;
static Mailer& get() {
static Mailer instance;
return instance;
}
void send(const std::string& text) { sent.push_back(text); } // a real mailer would call an email service here
};
struct Result {
int total;
std::string status;
};
class CheckoutService {
public:
Result place(const std::vector<Line>& items, const std::string& coupon, const std::string& payment) {
const Config& cfg = Config::get();
int subtotal = 0;
for (const Line& line : items) subtotal += line.price * line.qty;
int discount;
if (coupon.empty()) {
discount = 0;
} else if (coupon.rfind("FLAT", 0) == 0) {
discount = std::min(std::stoi(coupon.substr(4)), subtotal);
} else if (coupon.rfind("OFF", 0) == 0) {
discount = subtotal * std::stoi(coupon.substr(3)) / 100;
} else {
throw std::invalid_argument("unknown coupon: " + coupon);
}
int taxable = subtotal - discount;
int delivery = taxable >= cfg.freeDeliveryFrom ? 0 : cfg.deliveryFee;
int tax = taxable * cfg.taxPercent / 100;
int beforeFee = taxable + delivery + tax;
int fee;
if (payment == "card") {
fee = beforeFee * 2 / 100;
} else if (payment == "upi") {
fee = 0;
} else if (payment == "cod") {
fee = 20;
} else {
throw std::invalid_argument("unknown payment: " + payment);
}
int total = beforeFee + fee;
std::string status = payment == "cod" ? "pending" : "paid";
Mailer::get().send("receipt: " + std::to_string(total));
return {total, status};
}
};
int main() {
std::vector<Line> desk = {{"notebook", 120, 2}, {"pen", 25, 4}};
std::vector<Line> bag = {{"bag", 800, 1}};
std::vector<Line> both = {{"bag", 800, 1}, {"pen", 25, 4}};
CheckoutService service;
Result a = service.place(desk, "", "upi");
std::cout << "A: " << a.total << " " << a.status << "\n";
Result b = service.place(desk, "FLAT50", "card");
std::cout << "B: " << b.total << " " << b.status << "\n";
Result c = service.place(bag, "OFF10", "cod");
std::cout << "C: " << c.total << " " << c.status << "\n";
Result d = service.place(both, "FLAT1000", "upi");
std::cout << "D: " << d.total << " " << d.status << "\n";
try {
service.place(desk, "GOLD", "upi");
} catch (const std::invalid_argument& e) {
std::cout << "rejected: " << e.what() << "\n";
}
try {
service.place(desk, "", "cheque");
} catch (const std::invalid_argument& e) {
std::cout << "rejected: " << e.what() << "\n";
}
std::cout << "receipts sent: " << Mailer::get().sent.size() << "\n";
Config::get().taxPercent = 10; // one test changes a setting...
Result again = service.place(desk, "", "upi"); // ...and every later caller sees it
std::cout << "after one test sets tax to 10, order A costs " << again.total << "\n";
}// checkoutV1.js: one class that prices, charges, emails and reads two globals
class Config {
static #instance;
static get() {
if (!Config.#instance) Config.#instance = new Config();
return Config.#instance;
}
taxPercent = 5;
freeDeliveryFrom = 500;
deliveryFee = 40;
}
class Mailer {
static #instance;
static get() {
if (!Mailer.#instance) Mailer.#instance = new Mailer();
return Mailer.#instance;
}
sent = [];
send(text) {
this.sent.push(text); // a real mailer would call an email service here
}
}
class CheckoutService {
place(items, coupon, payment) {
const cfg = Config.get();
let subtotal = 0;
for (const { price, qty } of items) subtotal += price * qty;
let discount;
if (coupon === "") {
discount = 0;
} else if (coupon.startsWith("FLAT")) {
discount = Math.min(Number(coupon.slice(4)), subtotal);
} else if (coupon.startsWith("OFF")) {
discount = Math.floor((subtotal * Number(coupon.slice(3))) / 100);
} else {
throw new Error("unknown coupon: " + coupon);
}
const taxable = subtotal - discount;
const delivery = taxable >= cfg.freeDeliveryFrom ? 0 : cfg.deliveryFee;
const tax = Math.floor((taxable * cfg.taxPercent) / 100);
const beforeFee = taxable + delivery + tax;
let fee;
if (payment === "card") {
fee = Math.floor((beforeFee * 2) / 100);
} else if (payment === "upi") {
fee = 0;
} else if (payment === "cod") {
fee = 20;
} else {
throw new Error("unknown payment: " + payment);
}
const total = beforeFee + fee;
const status = payment === "cod" ? "pending" : "paid";
Mailer.get().send("receipt: " + total);
return { total, status };
}
}
const desk = [{ name: "notebook", price: 120, qty: 2 }, { name: "pen", price: 25, qty: 4 }];
const bag = [{ name: "bag", price: 800, qty: 1 }];
const both = [{ name: "bag", price: 800, qty: 1 }, { name: "pen", price: 25, qty: 4 }];
const service = new CheckoutService();
const orders = [
["A", desk, "", "upi"],
["B", desk, "FLAT50", "card"],
["C", bag, "OFF10", "cod"],
["D", both, "FLAT1000", "upi"],
];
for (const [label, items, coupon, payment] of orders) {
const r = service.place(items, coupon, payment);
console.log(`${label}: ${r.total} ${r.status}`);
}
for (const [items, coupon, payment] of [[desk, "GOLD", "upi"], [desk, "", "cheque"]]) {
try {
service.place(items, coupon, payment);
} catch (e) {
console.log("rejected:", e.message);
}
}
console.log("receipts sent:", Mailer.get().sent.length);
Config.get().taxPercent = 10; // one test changes a setting...
const again = service.place(desk, "", "upi"); // ...and every later caller sees it
console.log("after one test sets tax to 10, order A costs", again.total);A: 397 paid
B: 350 paid
C: 776 pending
D: 40 paid
rejected: unknown coupon: GOLD
rejected: unknown payment: cheque
receipts sent: 4
after one test sets tax to 10, order A costs 414The rules, so the numbers make sense. The subtotal is price times quantity, summed. A coupon FLAT50 takes 50 rupees off (never more than the subtotal) and OFF10 takes 10 percent off, rounded down. Delivery is 40 rupees, free once the discounted amount reaches 500. Tax is 5 percent of the discounted amount, rounded down. Then the payment fee: 2 percent for a card, nothing for UPI, 20 rupees for cash on delivery. Cash orders are pending; the rest are paid.
Check order A by hand. Two notebooks at 120 and four pens at 25 make 340. No coupon, so the discounted amount is 340, which is under 500, so delivery is 40. Tax is 340 × 5 / 100 = 17. That is 397, UPI adds nothing, and the total is 397.
The last line of the output is the quiet one. One test sets the tax to 10 percent, and the next order, which has nothing to do with that test, is charged 414 instead of 397. The setting lives in a global that every caller shares.
Nothing here is wrong today. It runs, and the numbers are right. The trouble is what the next year does to it:
- Every new payment method edits
place, the one method that also prices every order, so a typo in the wallet branch can break card orders. - Nothing can be tested alone.
placereadsConfig.get()andMailer.get()from inside, so to try a 10 percent tax you change a global that every other test also sees, and every call sends a receipt whether you wanted one or not. - Strings carry the rules.
"FLAT50"is cut at position 4 and read as a number;"cod"is compared in two places, once for the fee and once for the status, and a different spelling in one of them gives a wrong status without an error. - Four teams edit one class. Finance owns the tax and the delivery settings, marketing owns coupons, payments owns the methods and operations owns the receipt.
- The rules are nowhere to be read. To answer "what does a coupon do?" you read
placefrom top to bottom.
Count the reasons to change
List who might ask for a change to place in the next year. How many separate reasons does this one class have to change?
One strong answer
At least four. Finance changes the tax or the free-delivery limit, marketing adds a coupon type, the payments team adds a method or changes a fee, and operations changes what the receipt says or how it is sent. A class with several reasons to change is what the single responsibility principle calls too big, and the smell for it is the god class. The refactor splits the class along those lines.
Five smells, and which lesson cures each#
A smell is a hint, not a rule: it tells you where to look, and sometimes you look and leave it. These are the five this lesson works through:
| Smell | In the first version | The cure | Lesson |
|---|---|---|---|
| God class | CheckoutService.place prices, parses, charges and mails |
Split by reason to change | dp-03 |
| If-ladder on a kind | if coupon.startswith(...) and if payment == ... inside the price |
One small class per kind, one place that chooses | dp-16 |
| Primitive obsession | "FLAT50", "cod" and "paid" are plain strings; the rule is string slicing |
Parse once into a value; use an enum for status | dp-01, dp-19 |
| Singleton abuse | Config.get() and Mailer.get() are inputs hidden inside a method |
Pass them in | dp-06, dp-04 |
| Over-patterning | Not here yet: it arrives when a cure is applied to a rule that never varies | Stop earlier | dp-05 |
The fifth is the odd one out: the first four are what the code has, and the fifth is what you add when you get enthusiastic.
Refactor in small steps, under a pin#
A clean-up is a change, and every change can break something. The answer is not to be careful. It is to pin the behaviour first: run the code on a handful of orders, write down what it prints, and check against that recording after every step. This is often called a characterisation test. It does not say the old code is right, only that the refactor must not change it.
Think of moving the furniture in a room. Before you touch anything you take a photo. After every move you hold the photo up to the room. If a chair is missing you don't have to guess which of the last twenty moves lost it, because you only made one.
In this lesson the pin is four orders, chosen to hit each branch (a plain order, a flat coupon with a card, a percentage coupon with cash, a coupon larger than the cart), plus the two bad inputs. Order D is the one worth a second look: a 1000-rupee coupon on a 900-rupee cart takes off 900, not 1000, and the customer still pays the 40-rupee delivery. A rewrite could easily lose that. The pin remembers it.
The six steps, each ending with the pin green:
- Pin the behaviour. Run the first version on the four orders and the two bad ones, and write down every total. This is the only step that adds a test, and it changes no line of the service.
- Pass the globals in.
Config.get()becomes aSettingsvalue andMailer.get()becomes aNotifier, both given to the constructor. Nothing else moves. (dp-04) - Pull the price out. Everything that only calculates moves into
PriceCalculator.total, which has no mail and no globals.placekeeps the orchestration. (dp-03) - Parse the coupon once. The string slicing becomes
parse_coupon(code), which returnsFlatOfforPercentOff, and the calculator callscoupon.discount(subtotal)with noifon a code. (dp-16) - Stop. Read the result and ask of each class whether anything in it changes for a different reason than the rest. If not, you are done. (dp-05)
The order matters. The globals come first because, while place reads Config.get(), you cannot run the calculation with a different setting without touching shared state. The calculator comes before the strategies because a strategy needs a stable place to be called from. And no step changes behaviour and structure together: if you notice a bug on the way, write it down and fix it afterwards, with the pin updated on purpose.
Six small steps, each ending with the pin green: pin, pass the globals in, pull the price out, parse the coupon, table the payments, stop.
The code after the refactor#
Here is the result. The coupon and the payment method are small interfaces; the price is a class of its own; CheckoutService is given what it needs, so a test hands it a fake notifier:
# checkout.py: the same checkout, split into small parts with their dependencies passed in
from abc import ABC, abstractmethod
from dataclasses import dataclass
from enum import Enum
from typing import List
@dataclass(frozen=True)
class Line:
name: str
price: int # rupees
qty: int
@dataclass(frozen=True)
class Settings: # a value passed in, not a global read from inside
tax_percent: int = 5
free_delivery_from: int = 500
delivery_fee: int = 40
class Status(Enum):
PAID = "paid"
PENDING = "pending"
class Coupon(ABC):
@abstractmethod
def discount(self, subtotal: int) -> int: ...
class NoCoupon(Coupon):
def discount(self, subtotal: int) -> int:
return 0
class FlatOff(Coupon):
def __init__(self, amount: int) -> None:
self.amount = amount
def discount(self, subtotal: int) -> int:
return min(self.amount, subtotal)
class PercentOff(Coupon):
def __init__(self, percent: int) -> None:
self.percent = percent
def discount(self, subtotal: int) -> int:
return subtotal * self.percent // 100
def parse_coupon(code: str) -> Coupon: # the only place that reads the code string
if code == "":
return NoCoupon()
if code.startswith("FLAT"):
return FlatOff(int(code[4:]))
if code.startswith("OFF"):
return PercentOff(int(code[3:]))
raise ValueError("unknown coupon: " + code)
class PaymentMethod(ABC):
status = Status.PAID
@abstractmethod
def fee(self, amount: int) -> int: ...
class Card(PaymentMethod):
def fee(self, amount: int) -> int:
return amount * 2 // 100
class Upi(PaymentMethod):
def fee(self, amount: int) -> int:
return 0
class CashOnDelivery(PaymentMethod):
status = Status.PENDING
def fee(self, amount: int) -> int:
return 20
PAYMENTS = {"card": Card(), "upi": Upi(), "cod": CashOnDelivery()} # the one chooser
def payment_for(name: str) -> PaymentMethod:
if name not in PAYMENTS:
raise ValueError("unknown payment: " + name)
return PAYMENTS[name]
class Notifier(ABC):
@abstractmethod
def send(self, text: str) -> None: ...
class PriceCalculator:
"""Pure: no mail, no globals. Same inputs, same total."""
def __init__(self, settings: Settings) -> None:
self.settings = settings
def total(self, lines: List[Line], coupon: Coupon, payment: PaymentMethod) -> int:
subtotal = sum(line.price * line.qty for line in lines)
taxable = subtotal - coupon.discount(subtotal)
s = self.settings
delivery = 0 if taxable >= s.free_delivery_from else s.delivery_fee
before_fee = taxable + delivery + taxable * s.tax_percent // 100
return before_fee + payment.fee(before_fee)
@dataclass(frozen=True)
class Receipt:
total: int
status: Status
class CheckoutService:
"""Orchestrates: price it, then tell the customer."""
def __init__(self, calculator: PriceCalculator, notifier: Notifier) -> None:
self.calculator, self.notifier = calculator, notifier
def place(self, lines: List[Line], coupon_code: str, payment_name: str) -> Receipt:
coupon, payment = parse_coupon(coupon_code), payment_for(payment_name)
total = self.calculator.total(lines, coupon, payment)
self.notifier.send("receipt: " + str(total))
return Receipt(total, payment.status)
class RecordingNotifier(Notifier): # a test double: no email service needed
def __init__(self) -> None:
self.sent: List[str] = []
def send(self, text: str) -> None:
self.sent.append(text)
if __name__ == "__main__":
desk = [Line("notebook", 120, 2), Line("pen", 25, 4)]
bag = [Line("bag", 800, 1)]
both = [Line("bag", 800, 1), Line("pen", 25, 4)]
notifier = RecordingNotifier()
service = CheckoutService(PriceCalculator(Settings()), notifier)
orders = [
("A", desk, "", "upi"),
("B", desk, "FLAT50", "card"),
("C", bag, "OFF10", "cod"),
("D", both, "FLAT1000", "upi"),
]
recorded = {"A": 397, "B": 350, "C": 776, "D": 40} # totals the first version printed
matches = 0
for label, lines, coupon, payment in orders:
r = service.place(lines, coupon, payment)
print(f"{label}: {r.total} {r.status.value}")
matches += r.total == recorded[label]
for lines, coupon, payment in [(desk, "GOLD", "upi"), (desk, "", "cheque")]:
try:
service.place(lines, coupon, payment)
except ValueError as e:
print("rejected:", e)
print(f"pinned: {matches} of {len(recorded)} totals match the first version")
print("receipts sent:", len(notifier.sent))
class Wallet(PaymentMethod): # a fourth way to pay: one class, one line in the table
def fee(self, amount: int) -> int:
return 0
PAYMENTS["wallet"] = Wallet()
r = service.place(desk, "", "wallet")
print(f"wallet (new): {r.total} {r.status.value}")
strict = CheckoutService(PriceCalculator(Settings(tax_percent=10)), RecordingNotifier())
own, default = strict.place(desk, "", "upi").total, service.place(desk, "", "upi").total
print(f"tax 10 in its own calculator: {own}; the default one: {default}")// Checkout.java: the same checkout, split into small parts with their dependencies passed in
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
class Checkout {
record Line(String name, int price, int qty) {} // rupees
record Settings(int taxPercent, int freeDeliveryFrom, int deliveryFee) {} // a value passed in, not a global read from inside
record Receipt(int total, Status status) {}
enum Status {
PAID("paid"), PENDING("pending");
final String label;
Status(String label) { this.label = label; }
}
interface Coupon {
int discount(int subtotal);
}
static final class NoCoupon implements Coupon {
public int discount(int subtotal) { return 0; }
}
static final class FlatOff implements Coupon {
private final int amount;
FlatOff(int amount) { this.amount = amount; }
public int discount(int subtotal) { return Math.min(amount, subtotal); }
}
static final class PercentOff implements Coupon {
private final int percent;
PercentOff(int percent) { this.percent = percent; }
public int discount(int subtotal) { return subtotal * percent / 100; }
}
static Coupon parseCoupon(String code) { // the only place that reads the code string
if (code.isEmpty()) return new NoCoupon();
if (code.startsWith("FLAT")) return new FlatOff(Integer.parseInt(code.substring(4)));
if (code.startsWith("OFF")) return new PercentOff(Integer.parseInt(code.substring(3)));
throw new IllegalArgumentException("unknown coupon: " + code);
}
interface PaymentMethod {
int fee(int amount);
default Status status() { return Status.PAID; }
}
static final class Card implements PaymentMethod {
public int fee(int amount) { return amount * 2 / 100; }
}
static final class Upi implements PaymentMethod {
public int fee(int amount) { return 0; }
}
static final class CashOnDelivery implements PaymentMethod {
public int fee(int amount) { return 20; }
public Status status() { return Status.PENDING; }
}
static final Map<String, PaymentMethod> PAYMENTS = new HashMap<>(Map.of(
"card", new Card(), "upi", new Upi(), "cod", new CashOnDelivery())); // the one chooser
static PaymentMethod paymentFor(String name) {
PaymentMethod p = PAYMENTS.get(name);
if (p == null) throw new IllegalArgumentException("unknown payment: " + name);
return p;
}
interface Notifier {
void send(String text);
}
/** Pure: no mail, no globals. Same inputs, same total. */
static final class PriceCalculator {
private final Settings settings;
PriceCalculator(Settings settings) { this.settings = settings; }
int total(List<Line> lines, Coupon coupon, PaymentMethod payment) {
int subtotal = 0;
for (Line line : lines) subtotal += line.price() * line.qty();
int taxable = subtotal - coupon.discount(subtotal);
int delivery = taxable >= settings.freeDeliveryFrom() ? 0 : settings.deliveryFee();
int beforeFee = taxable + delivery + taxable * settings.taxPercent() / 100;
return beforeFee + payment.fee(beforeFee);
}
}
/** Orchestrates: price it, then tell the customer. */
static final class CheckoutService {
private final PriceCalculator calculator;
private final Notifier notifier;
CheckoutService(PriceCalculator calculator, Notifier notifier) {
this.calculator = calculator;
this.notifier = notifier;
}
Receipt place(List<Line> lines, String couponCode, String paymentName) {
Coupon coupon = parseCoupon(couponCode);
PaymentMethod payment = paymentFor(paymentName);
int total = calculator.total(lines, coupon, payment);
notifier.send("receipt: " + total);
return new Receipt(total, payment.status());
}
}
static final class RecordingNotifier implements Notifier { // a test double: no email service needed
final List<String> sent = new ArrayList<>();
public void send(String text) { sent.add(text); }
}
record Order(String label, List<Line> lines, String coupon, String payment) {}
static final class Wallet implements PaymentMethod { // a fourth way to pay: one class, one line in the table
public int fee(int amount) { return 0; }
}
public static void main(String[] args) {
List<Line> desk = List.of(new Line("notebook", 120, 2), new Line("pen", 25, 4));
List<Line> bag = List.of(new Line("bag", 800, 1));
List<Line> both = List.of(new Line("bag", 800, 1), new Line("pen", 25, 4));
RecordingNotifier notifier = new RecordingNotifier();
CheckoutService service = new CheckoutService(new PriceCalculator(new Settings(5, 500, 40)), notifier);
List<Order> orders = List.of(
new Order("A", desk, "", "upi"),
new Order("B", desk, "FLAT50", "card"),
new Order("C", bag, "OFF10", "cod"),
new Order("D", both, "FLAT1000", "upi"));
Map<String, Integer> recorded = Map.of("A", 397, "B", 350, "C", 776, "D", 40); // totals the first version printed
int matches = 0;
for (Order o : orders) {
Receipt r = service.place(o.lines(), o.coupon(), o.payment());
System.out.println(o.label() + ": " + r.total() + " " + r.status().label);
if (r.total() == recorded.get(o.label())) matches++;
}
try {
service.place(desk, "GOLD", "upi");
} catch (IllegalArgumentException e) {
System.out.println("rejected: " + e.getMessage());
}
try {
service.place(desk, "", "cheque");
} catch (IllegalArgumentException e) {
System.out.println("rejected: " + e.getMessage());
}
System.out.println("pinned: " + matches + " of " + recorded.size() + " totals match the first version");
System.out.println("receipts sent: " + notifier.sent.size());
PAYMENTS.put("wallet", new Wallet());
Receipt r = service.place(desk, "", "wallet");
System.out.println("wallet (new): " + r.total() + " " + r.status().label);
CheckoutService strict = new CheckoutService(new PriceCalculator(new Settings(10, 500, 40)), new RecordingNotifier());
System.out.println("tax 10 in its own calculator: " + strict.place(desk, "", "upi").total() + "; the default one: " + service.place(desk, "", "upi").total());
}
}// checkout.cpp: the same checkout, split into small parts with their dependencies passed in
#include <iostream>
#include <map>
#include <memory>
#include <stdexcept>
#include <string>
#include <vector>
struct Line {
std::string name;
int price; // rupees
int qty;
};
struct Settings { // a value passed in, not a global read from inside
int taxPercent = 5;
int freeDeliveryFrom = 500;
int deliveryFee = 40;
};
enum class Status { Paid, Pending };
const char* label(Status s) { return s == Status::Paid ? "paid" : "pending"; }
class Coupon {
public:
virtual ~Coupon() = default;
virtual int discount(int subtotal) const = 0;
};
class NoCoupon : public Coupon {
public:
int discount(int) const override { return 0; }
};
class FlatOff : public Coupon {
int amount_;
public:
explicit FlatOff(int amount) : amount_(amount) {}
int discount(int subtotal) const override { return std::min(amount_, subtotal); }
};
class PercentOff : public Coupon {
int percent_;
public:
explicit PercentOff(int percent) : percent_(percent) {}
int discount(int subtotal) const override { return subtotal * percent_ / 100; }
};
// The only place that reads the code string.
std::unique_ptr<Coupon> parseCoupon(const std::string& code) {
if (code.empty()) return std::make_unique<NoCoupon>();
if (code.rfind("FLAT", 0) == 0) return std::make_unique<FlatOff>(std::stoi(code.substr(4)));
if (code.rfind("OFF", 0) == 0) return std::make_unique<PercentOff>(std::stoi(code.substr(3)));
throw std::invalid_argument("unknown coupon: " + code);
}
class PaymentMethod {
public:
virtual ~PaymentMethod() = default;
virtual int fee(int amount) const = 0;
virtual Status status() const { return Status::Paid; }
};
class Card : public PaymentMethod {
public:
int fee(int amount) const override { return amount * 2 / 100; }
};
class Upi : public PaymentMethod {
public:
int fee(int) const override { return 0; }
};
class CashOnDelivery : public PaymentMethod {
public:
int fee(int) const override { return 20; }
Status status() const override { return Status::Pending; }
};
// The one chooser.
std::map<std::string, std::shared_ptr<PaymentMethod>>& payments() {
static std::map<std::string, std::shared_ptr<PaymentMethod>> table = {
{"card", std::make_shared<Card>()},
{"upi", std::make_shared<Upi>()},
{"cod", std::make_shared<CashOnDelivery>()},
};
return table;
}
const PaymentMethod& paymentFor(const std::string& name) {
auto it = payments().find(name);
if (it == payments().end()) throw std::invalid_argument("unknown payment: " + name);
return *it->second;
}
class Notifier {
public:
virtual ~Notifier() = default;
virtual void send(const std::string& text) = 0;
};
// Pure: no mail, no globals. Same inputs, same total.
class PriceCalculator {
Settings settings_;
public:
explicit PriceCalculator(Settings settings) : settings_(settings) {}
int total(const std::vector<Line>& lines, const Coupon& coupon, const PaymentMethod& payment) const {
int subtotal = 0;
for (const Line& line : lines) subtotal += line.price * line.qty;
int taxable = subtotal - coupon.discount(subtotal);
int delivery = taxable >= settings_.freeDeliveryFrom ? 0 : settings_.deliveryFee;
int beforeFee = taxable + delivery + taxable * settings_.taxPercent / 100;
return beforeFee + payment.fee(beforeFee);
}
};
struct Receipt {
int total;
Status status;
};
// Orchestrates: price it, then tell the customer.
class CheckoutService {
const PriceCalculator& calculator_;
Notifier& notifier_;
public:
CheckoutService(const PriceCalculator& calculator, Notifier& notifier) : calculator_(calculator), notifier_(notifier) {}
Receipt place(const std::vector<Line>& lines, const std::string& couponCode, const std::string& paymentName) {
std::unique_ptr<Coupon> coupon = parseCoupon(couponCode);
const PaymentMethod& payment = paymentFor(paymentName);
int total = calculator_.total(lines, *coupon, payment);
notifier_.send("receipt: " + std::to_string(total));
return {total, payment.status()};
}
};
class RecordingNotifier : public Notifier { // a test double: no email service needed
public:
std::vector<std::string> sent;
void send(const std::string& text) override { sent.push_back(text); }
};
struct Order {
std::string label;
std::vector<Line> lines;
std::string coupon;
std::string payment;
};
class Wallet : public PaymentMethod { // a fourth way to pay: one class, one line in the table
public:
int fee(int) const override { return 0; }
};
int main() {
std::vector<Line> desk = {{"notebook", 120, 2}, {"pen", 25, 4}};
std::vector<Line> bag = {{"bag", 800, 1}};
std::vector<Line> both = {{"bag", 800, 1}, {"pen", 25, 4}};
RecordingNotifier notifier;
PriceCalculator calculator{Settings{}};
CheckoutService service(calculator, notifier);
std::vector<Order> orders = {
{"A", desk, "", "upi"},
{"B", desk, "FLAT50", "card"},
{"C", bag, "OFF10", "cod"},
{"D", both, "FLAT1000", "upi"},
};
std::map<std::string, int> recorded = {{"A", 397}, {"B", 350}, {"C", 776}, {"D", 40}}; // totals the first version printed
int matches = 0;
for (const Order& o : orders) {
Receipt r = service.place(o.lines, o.coupon, o.payment);
std::cout << o.label << ": " << r.total << " " << label(r.status) << "\n";
if (r.total == recorded[o.label]) matches++;
}
for (const auto& bad : std::vector<std::pair<std::string, std::string>>{{"GOLD", "upi"}, {"", "cheque"}}) {
try {
service.place(desk, bad.first, bad.second);
} catch (const std::invalid_argument& e) {
std::cout << "rejected: " << e.what() << "\n";
}
}
std::cout << "pinned: " << matches << " of " << recorded.size() << " totals match the first version\n";
std::cout << "receipts sent: " << notifier.sent.size() << "\n";
payments()["wallet"] = std::make_shared<Wallet>();
Receipt r = service.place(desk, "", "wallet");
std::cout << "wallet (new): " << r.total << " " << label(r.status) << "\n";
RecordingNotifier quiet;
PriceCalculator tenPercent{Settings{10, 500, 40}};
CheckoutService strict(tenPercent, quiet);
std::cout << "tax 10 in its own calculator: " << strict.place(desk, "", "upi").total << "; the default one: " << service.place(desk, "", "upi").total << "\n";
}// checkout.js: the same checkout, split into small parts with their dependencies passed in
// A line is { name, price, qty } with whole rupees. Settings is a value passed in, not a global read from inside.
const Status = Object.freeze({ PAID: "paid", PENDING: "pending" });
class NoCoupon {
discount(subtotal) {
return 0;
}
}
class FlatOff {
constructor(amount) {
this.amount = amount;
}
discount(subtotal) {
return Math.min(this.amount, subtotal);
}
}
class PercentOff {
constructor(percent) {
this.percent = percent;
}
discount(subtotal) {
return Math.floor((subtotal * this.percent) / 100);
}
}
// The only place that reads the code string.
function parseCoupon(code) {
if (code === "") return new NoCoupon();
if (code.startsWith("FLAT")) return new FlatOff(Number(code.slice(4)));
if (code.startsWith("OFF")) return new PercentOff(Number(code.slice(3)));
throw new Error("unknown coupon: " + code);
}
// A payment method is any object with fee(amount) and a status.
class Card {
status = Status.PAID;
fee(amount) {
return Math.floor((amount * 2) / 100);
}
}
class Upi {
status = Status.PAID;
fee(amount) {
return 0;
}
}
class CashOnDelivery {
status = Status.PENDING;
fee(amount) {
return 20;
}
}
// The one chooser.
const PAYMENTS = new Map([
["card", new Card()],
["upi", new Upi()],
["cod", new CashOnDelivery()],
]);
function paymentFor(name) {
if (!PAYMENTS.has(name)) throw new Error("unknown payment: " + name);
return PAYMENTS.get(name);
}
/** Pure: no mail, no globals. Same inputs, same total. */
class PriceCalculator {
constructor(settings) {
this.settings = settings;
}
total(lines, coupon, payment) {
const subtotal = lines.reduce((sum, line) => sum + line.price * line.qty, 0);
const taxable = subtotal - coupon.discount(subtotal);
const s = this.settings;
const delivery = taxable >= s.freeDeliveryFrom ? 0 : s.deliveryFee;
const beforeFee = taxable + delivery + Math.floor((taxable * s.taxPercent) / 100);
return beforeFee + payment.fee(beforeFee);
}
}
/** Orchestrates: price it, then tell the customer. */
class CheckoutService {
constructor(calculator, notifier) {
this.calculator = calculator;
this.notifier = notifier;
}
place(lines, couponCode, paymentName) {
const coupon = parseCoupon(couponCode);
const payment = paymentFor(paymentName);
const total = this.calculator.total(lines, coupon, payment);
this.notifier.send("receipt: " + total);
return { total, status: payment.status };
}
}
class RecordingNotifier {
// a test double: no email service needed
sent = [];
send(text) {
this.sent.push(text);
}
}
const desk = [{ name: "notebook", price: 120, qty: 2 }, { name: "pen", price: 25, qty: 4 }];
const bag = [{ name: "bag", price: 800, qty: 1 }];
const both = [{ name: "bag", price: 800, qty: 1 }, { name: "pen", price: 25, qty: 4 }];
const notifier = new RecordingNotifier();
const service = new CheckoutService(new PriceCalculator({ taxPercent: 5, freeDeliveryFrom: 500, deliveryFee: 40 }), notifier);
const orders = [
["A", desk, "", "upi"],
["B", desk, "FLAT50", "card"],
["C", bag, "OFF10", "cod"],
["D", both, "FLAT1000", "upi"],
];
const recorded = { A: 397, B: 350, C: 776, D: 40 }; // totals the first version printed
let matches = 0;
for (const [label, lines, coupon, payment] of orders) {
const r = service.place(lines, coupon, payment);
console.log(`${label}: ${r.total} ${r.status}`);
if (r.total === recorded[label]) matches++;
}
for (const [coupon, payment] of [["GOLD", "upi"], ["", "cheque"]]) {
try {
service.place(desk, coupon, payment);
} catch (e) {
console.log("rejected:", e.message);
}
}
console.log(`pinned: ${matches} of ${Object.keys(recorded).length} totals match the first version`);
console.log("receipts sent:", notifier.sent.length);
class Wallet {
// a fourth way to pay: one class, one line in the table
status = Status.PAID;
fee(amount) {
return 0;
}
}
PAYMENTS.set("wallet", new Wallet());
const r = service.place(desk, "", "wallet");
console.log(`wallet (new): ${r.total} ${r.status}`);
const strict = new CheckoutService(new PriceCalculator({ taxPercent: 10, freeDeliveryFrom: 500, deliveryFee: 40 }), new RecordingNotifier());
const own = strict.place(desk, "", "upi").total;
const standard = service.place(desk, "", "upi").total;
console.log(`tax 10 in its own calculator: ${own}; the default one: ${standard}`);All four print the same thing:
A: 397 paid
B: 350 paid
C: 776 pending
D: 40 paid
rejected: unknown coupon: GOLD
rejected: unknown payment: cheque
pinned: 4 of 4 totals match the first version
receipts sent: 4
wallet (new): 397 paid
tax 10 in its own calculator: 414; the default one: 397The pinned line is the pin from step 1, now living in the program: the four totals the first version printed are written into the code as literal numbers, and all four match. The last two lines are what the refactor bought. A fourth payment method, wallet, was added by one class and one line in the table, and no existing class was edited. And two calculators with different settings live side by side in one process, where the first version had one global that every caller shared.
Check order B by hand. Subtotal 340. FLAT50 takes off 50, so 290. That is under 500, so delivery is 40. Tax is 290 × 5 / 100 = 14 (14.5, rounded down). That is 344. A card charges 2 percent of 344, which is 6 (6.88, rounded down). The total is 350, the same as the first version.
%% title: The checkout after the refactor
classDiagram
class CheckoutService {
+place(lines, couponCode, paymentName) Receipt
}
class PriceCalculator {
+total(lines, coupon, payment) int
}
class Settings {
+tax_percent int
+free_delivery_from int
+delivery_fee int
}
class Coupon {
<<interface>>
+discount(subtotal) int
}
class FlatOff
class PercentOff
class PaymentMethod {
<<interface>>
+fee(amount) int
}
class Card
class CashOnDelivery
class Notifier {
<<interface>>
+send(text)
}
CheckoutService --> PriceCalculator : asks
CheckoutService --> Notifier : tells
PriceCalculator --> Settings : reads
PriceCalculator ..> Coupon : calls
PriceCalculator ..> PaymentMethod : calls
Coupon <|.. FlatOff
Coupon <|.. PercentOff
PaymentMethod <|.. Card
PaymentMethod <|.. CashOnDeliveryRead the arrows. CheckoutService points at the calculator and the notifier, and nothing else: it has no idea how a coupon works. PriceCalculator points at two interfaces, never at FlatOff or Card. The dotted arrows with hollow heads are "implements". The diagram leaves out NoCoupon, Upi and RecordingNotifier to stay small. Every arrow ends at a thing a test or the next requirement can replace.
What each language brings to the same design:
- Python: frozen dataclasses for
Line,SettingsandReceipt, anEnumforStatus, andABCfor the interfaces. A mistypedStatus.PAID_fails when that line runs. - Java:
records for the value types, anenumwith a label, and an interface with a default method, sostatus()isPAIDunless a method says otherwise. The payment table is a mutable map only so the demo can registerwallet. - C++:
enum class Status, andparseCouponreturns aunique_ptr<Coupon>because it creates a new object each time, while the payment table holdsshared_ptrs because the table owns them and outlives every order. - JavaScript: no interfaces and no enum. A coupon is any object with
discount, a payment method is any object withfeeandstatus, andObject.freezemakesStatusread-only. A misspeltStatus.PAID_isundefinedat run time.
The refactor is longer, and that is worth saying plainly. With their demo code, the Python programs are 87 lines before and 178 after, and 3 classes became 16. You paid for thirteen more small classes, and what you got back is the three things the first version could not do: price an order without sending mail, run with a different setting without touching a global, and add a payment method without opening place.
The if on payment names did not vanish: it left the middle of the price calculation and became one lookup at the edge.
When the cure is worse: over-patterning#
Now the fifth smell. Tax is 5 percent, finance mentioned once that it might differ by state one day, and the pattern-minded part of you sees a strategy:
# tax_overdone.py: a strategy, a factory and a provider for one rule that has never changed
from abc import ABC, abstractmethod
class TaxStrategy(ABC):
@abstractmethod
def tax(self, taxable: int) -> int: ...
class PercentTaxStrategy(TaxStrategy):
def __init__(self, percent: int) -> None:
self.percent = percent
def tax(self, taxable: int) -> int:
return taxable * self.percent // 100
class TaxStrategyFactory:
def create(self, kind: str, percent: int) -> TaxStrategy:
if kind == "percent":
return PercentTaxStrategy(percent)
raise ValueError("unknown tax kind: " + kind)
class TaxStrategyProvider:
def __init__(self, factory: TaxStrategyFactory, percent: int) -> None:
self.factory, self.percent = factory, percent
def get(self) -> TaxStrategy:
return self.factory.create("percent", self.percent)
print(TaxStrategyProvider(TaxStrategyFactory(), 5).get().tax(340)) # 17It prints 17, the same as the one line it replaces in PriceCalculator: taxable * settings.tax_percent // 100. The replacement is 33 lines and four types, and the factory has an if on a kind that has exactly one value. Nothing varies, so nothing is gained, and a reader who meets TaxStrategyProvider has to open three files to learn that tax is a percentage.
A number in settings is enough when
- Only the value changes (5 percent today, maybe 6 next year)
- There is one rule and no second rule in the requirements
- A test can set it with
Settings(tax_percent=10)
A strategy earns its place when
- The shape of the rule differs: a percentage here, a slab table there
- A second variant is already in the requirements, not in your imagination
- Different people own and test the variants
The test for the cure is the same one the Strategy lesson gives for the pattern: if a requirement added at minute 75 would make you edit the code that does the job, that job belongs behind an interface. Tax by state would be that requirement, and when it arrives the change is small and safe, which is what the pin is for. Until then a number in Settings is the honest design. The vending machine says this about change-making: if the operator wants a different rule, make_change becomes a strategy; say it, don't build it unasked.
A tax strategy, a factory and a provider print the same 17 as one line, in 33 lines and four types, because nothing varies.
A state-by-state tax
Finance now says tax will be 5 percent in one state and 12 percent in another, from next quarter. Does that justify a TaxStrategy today?
One strong answer
Not yet, and probably not as a strategy at all. Two percentages are still one shape of rule: a number that depends on the state. A small table from state to percent, held in Settings, handles it with no new class. A strategy appears when a second shape arrives, such as a rule that taxes by item category or by slab. Keep the pin, make the smallest change that fits, and refactor when the change stops fitting.
Smells and the patterns that look like them#
Some smells have respectable relatives. Telling them apart is most of the skill:
| It looks like a smell | It is fine when | The difference |
|---|---|---|
| A god class | it is a thin front door | A Facade (the Facade lesson) delegates every real job to other classes, as FoodService does in food delivery. A god class does the jobs itself. After the refactor, CheckoutService is close to a facade: it calls and tells. |
| A singleton | one instance is created at startup and passed in | The parking lot says to create one instance at startup and pass it around. The smell is the hidden .get() from inside a method, not the fact that there is only one. |
| Strings and numbers where a class could be | the value is just a value | Money as whole rupees is a number and that is fine. It becomes a smell when rules hang off the string, like the position-4 cut in "FLAT50". |
An if on a kind |
there are two fixed cases that will never grow | Ascending or descending is an if. A payment method is a family. |
When not to refactor#
- It works and it never changes. A 60-line report script that runs once a quarter can be ugly. Cleaning it up is a cost with no return.
- You can't pin it and have no time to. Without a recording, a refactor is a rewrite with extra steps. Pin first, even if the pin is three orders.
- A feature or a fix is waiting. Do the change first, in the old shape, or refactor first and then change; never both in one commit, because then a red pin could mean either.
- It's the interview, and the clock is running. In a 90-minute round you design the parts first. Refactoring a messy class is a thing to talk through, not to demo.
Where it shows up in the case studies#
Every case study ends with a "Common mistakes" panel, and most of them are these smells under another name. A sample:
| Case study | The mistake it names | What the round does instead |
|---|---|---|
| Parking lot | A ParkingLot god class with the pricing if/elif, the spot search and the printing inside park() |
Two strategies, ParkingLot(floors, assigner, pricing); Singleton named as avoided |
| Rate limiter | One class with an algorithm string and an if/elif per method |
One class per algorithm, and an injected Clock |
| Splitwise | A split_type string and an if/elif ladder inside add_expense |
SplitStrategy |
| Vending machine | One VendingMachine with if self.state == "IDLE" in every method |
A class per state; strategy for change-making left as "say it, don't build it" |
| Snake and ladder | random.randint called inside the game |
A Dice passed in, so a seed replays a game |
| Pub-sub logger | "Patterns as decoration": a chain where every link just calls the next | Each pattern has a job, or it is left out |
| Ride sharing | Fare logic inside Trip |
FareStrategy, wrapped by SurgeFare |
| File system | isinstance checks in every operation |
A shared Node interface |
The case studies are the after, and their mistakes panels describe the before you are meant to avoid.
In the interview#
Say the diagnosis before the cure, in one breath: "This class has four reasons to change, so I'll split it along them: price, coupon, payment and notification. I'll only put a strategy where a second variant is already in the requirements, and I'll pass the settings and the notifier in so I can test the price alone." Naming the smell and the limit scores more than naming a pattern.
Then expect the follow-ups:
- "There are no tests. How do you refactor it safely?" Pin the behaviour first: run it on a few orders chosen to hit every branch, record the outputs, and rerun them after every step. If a step turns the pin red, undo that step only.
- "Should every
ifbecome a strategy?" No. Two fixed cases stay anif, and a rule whose only variation is a number stays a number in settings. A strategy needs a second shape of rule. - "Isn't passing everything in over-engineering?" Pass in what varies or what a test must fake: the clock, the mailer, the settings. Not every helper function.
- "How do you know when to stop?" When no class changes for two different reasons, and the next requirement on your list would land in one place. After that, more classes only add reading.
- "Why not rewrite it?" Because the old code encodes behaviour nobody wrote down. Order D is one: a coupon bigger than the cart takes off the cart, not the coupon. A pin keeps that; a rewrite quietly drops it.
Common mistakes#
- Refactoring with no pin. Then nobody can say whether the behaviour changed, so nobody can say the refactor is safe.
- Changing behaviour and structure in the same step. A red pin could then mean either, and you lose the point of the pin.
- A pattern for every
if. Four files for a rule that is always 5 percent. - Singletons "for convenience", then tests that pass alone and fail together because a global kept its last value.
- Calling it done when the ladder only moved. Moving the choice to one place at the edge is the goal; claiming the
ifdisappeared is not true.
Anti-patterns and refactoring
Q1/4 You've been asked to split a 400-line checkout class that has no tests. What is the first step?
Answer & explanation
Answer: B
Any change can break something, so first record what the code does today. That recording is the pin you check after every later step. Without it you cannot tell a safe refactor from a rewrite.
Q2/4 A method calls
Mailer.get().send(...)from inside. What is the main problem for testing it?Answer & explanation
Answer: B
The method depends on something it doesn't take as a parameter. Tests can't give it a fake, and state left by one test is visible to the next. Passing a
Notifierin removes both problems.Q3/4 Tax is always 5 percent, and finance says it might differ by state next quarter. What is the right design today?
Answer & explanation
Answer: B
One number that may change is a setting. A strategy is for a second shape of rule, and by then the change is small and safe because the pin is in place.
Q4/4 After the refactor, what happened to the if-ladder on payment names?
Answer & explanation
Answer: C
The decision still exists. It left the middle of the price calculation, where it ran for every order and had to be edited for every new method, and now runs once where a name from outside becomes an object.
this lesson in shorts · 3
play them all →One idea from this lesson in about a minute each, made for your phone.
New lessons by email
New lessons, side quests and case files as they ship, at most once a week. No spam, unsubscribe anytime.
Discussion · …
Public: everyone reading this lesson sees comments. Be kind and keep it on the material.