Practice Lab 30 — Refactoring a conditional into a design¶

Difficulty: Expert · Estimated time: 7 hours · Module 15: SOLID Principles and Design Patterns

A working program with one function that has grown for two years. Your job is to change its shape without changing its behaviour — and to prove both halves of that.

At a glance¶


Difficulty	Expert
Estimated time	7 hours
Module	Module 15 — SOLID Principles and Design Patterns
Concepts practised	Identifying SOLID violations, the open/closed principle, strategy and factory, interface segregation, dependency inversion, characterisation tests before refactoring, incremental change
Prerequisite	Practice Lab 29
Class design	yours — the existing behaviour is fixed, the structure is not

Scenario¶

A small payroll calculator works. It has for two years. It is 180 lines in one function:

for each employee:
    read the employee type field
    if type == "salaried":      compute salary / 12, minus deductions
    else if type == "hourly":   hours * rate, with overtime above 40
    else if type == "commission": base * 0.4 + weekly sales * rate
    else if type == "contractor": fixed monthly, no deductions, no benefits
    else:                       print "unknown type" and skip
    then handle deductions, which differ by the employee's location (3 places)
    then benefits, which only some types get
    then print a payslip in one of two layouts, chosen by an environment-ish flag

Two things are about to happen: two new employee types are being introduced next quarter, and the deduction rules are changing for one location. Both currently require editing this function, which is the whole problem.

Problem statement¶

Refactor the calculator, keeping behaviour identical and proving it.

A characterisation suite first: capture the existing program's output for a corpus of at least 25 employees, covering every type, every location, both payslip layouts, overtime boundaries (exactly 40 hours), zero hours, an unknown type, and an employee with no benefits. This output is the contract, saved before you touch anything.
The refactoring, in small steps, each step keeping the suite green:
Extract the type-dependent pay calculation behind an abstraction.
Extract the location-dependent deductions behind a second abstraction, without coupling it to the first.
Extract the payslip layout behind a third abstraction, chosen by the caller rather than read from inside.
Introduce a way to create the right employee type from the input field, so the calculator never sees a type string.
A SOLID audit in refactor-notes.md: for each of the five principles, the violation that was present, the change that removed it, and the code that still violates it (some will).
Two proofs:
Behaviour preserved: the suite's output is byte-identical before and after, and it is re-run after every step, with the step log recorded.
The goal achieved: adding the two new types touches one file and edits no existing calculation; adding a fourth location touches one file. Demonstrate by actually doing one of them in a branch copy, and record the diff.
An honest audit of the cost: what got harder, what got bigger, and where the new seam invites a bug that the old function could not have.

In refactor-notes.md:

The five principles, each with violation → change → what still violates it.
The step log: each step, what it changed, and the suite's result.
The two new type definitions and the diff, with file and line counts.
What each abstraction costs (files, indirection, one extra failure mode).
The thing you could not cleanly separate, and why.

Learning objectives¶

By the end of this lab you will be able to:

Capture a characterisation suite before refactoring, and use it as the safety net.
Refactor a conditional into polymorphic strategies without changing observable behaviour.
Separate three concerns that were tangled in one loop: pay, deductions, presentation.
Show, with a measured diff, that a new case no longer requires editing existing logic.
Report the cost of an abstraction honestly, including where it is not worth it.

Functional requirements¶

The suite runs before and after, is byte-identical, and is checked in as part of the program — an expected-output file or an assertion per case.
Each refactoring step is described in the log with its suite result; no step is skipped or squashed.
No branch on a type string remains in the calculation path — searched for and recorded.
Deductions and pay do not know about each other: neither abstraction's interface mentions the other's concepts.
The payslip layout is passed in by the caller; the calculator has no configuration read inside.
The employee type string is read in exactly one place.
Adding the two new types edits only the new files plus one registration or factory line — recorded.
Adding a fourth location edits only one file — recorded.
Every existing test case for overtime boundary, zero hours, unknown type and no benefits still passes, with the same output as before.
The unknown type behaviour is unchanged and now reported at one place; record whether the interface made that harder or easier.

Constraints¶

The original function is kept in the project as before/ for comparison — untouched, not deleted.
The suite's expected output is produced by running the original, not by hand.
No new behaviour, no "improvements" outside the stated goal, no renames for taste. If you change one thing beyond the goal, the notes must list it and justify it.
Automated tests are not available at this stage of the course, so the suite is a program with assertions and a captured expected output — that is the constraint, and it is enough if you keep it honest.
No warnings.

Starter task¶

In refactor-notes.md, before coding:

Read the original and list every decision it makes, in the order it makes them. This is the behaviour to preserve.
List the five principles and mark which one is violated most expensively, with the line you would have to edit.
Write the corpus: 25 employees, naming what each one covers.
Predict the size of the diff for adding a new type before the refactoring, in files and lines.
Write the one separation you expect to be hardest, and why.

Implementation tasks¶

Capture the corpus output from the original and save it.
Extract the pay calculation behind an abstraction; run the suite.
Extract the deductions behind a second abstraction; run the suite.
Extract the payslip layout and move its selection to the caller; run the suite.
Introduce the type creation point; run the suite.
Add the two new types on a copy; produce the diff and the counts.
Add the fourth location on a copy; produce the diff.
Write the five-principle audit, including what still violates each.

Test cases¶

#	Action	Expected result
1	Suite against `before/`	Captured; this is the contract
2	Suite after step 1	Identical
3	Suite after step 2	Identical
4	Suite after step 3	Identical
5	Suite after step 4	Identical
6	Exactly 40 hours, hourly	No overtime pay; identical to before
7	40.5 hours, hourly	Overtime on the excess only
8	Zero hours	The documented result, unchanged
9	Unknown type	Skipped with the same message, produced at one place
10	Contractor payslip	No deductions, no benefits, unchanged
11	Both payslip layouts over the whole corpus	Both identical to before
12	Add a new type on a copy	One file added plus one registration line — diff recorded
13	Add a fourth location on a copy	One file — diff recorded
14	Search for a type-string branch in the calculation path	Nothing found

Boundary cases to run deliberately: the corpus's first and last employees; the overtime boundary either side; a location with no items on the payslip; an employee whose type is unknown but whose location is valid.

Expected behaviour¶

suite vs before: 25 cases, 25 identical, 0 differing
step log:
  step 1 pay strategies      -> 25/25 identical
  step 2 deductions          -> 25/25 identical
  step 3 payslip layouts     -> 25/25 identical
  step 4 type creation       -> 25/25 identical
adding 'intern' and 'retainer':
  before refactoring: 1 file, ~38 lines edited
  after refactoring:  1 new file (29 lines) + 1 registration line
adding a fourth location:
  before: 1 file, ~22 lines edited
  after:  1 new file (34 lines) + 1 registration line
still violating: the unknown-type message lives in the factory, not the calculator -
  acceptable, and recorded

Extension challenge¶

The original function also prints as it calculates. Give the calculator a way to report the same information without printing, so that another caller can consume it — and keep the existing printing caller working unchanged.

Record in refactor-notes.md:

The interface for reporting, and how the printing caller is expressed in terms of it.
What the characterisation suite proves about the new path: is the printed output identical, and how do you know the non-printing path computed the same numbers?
Whether this is a refactoring or a new feature, in one sentence, and what the difference means for the suite.
The change cost: files, operations, lines.

Debugging challenge¶

After the refactoring, one corpus case changes: an hourly employee at exactly 40 hours now gets a small overtime amount.

double HourlyPay::compute(const Employee& e) const {
    const double regular = e.hours() * e.rate();
    const double overtime = (e.hours() - 40.0) * e.rate() * 1.5;
    return regular + overtime;          // negative overtime when hours < 40
}

Reproduce it — run the suite and find which cases move — and answer:

Which corpus cases change, and why does the boundary case at exactly 40 hide the sign of the bug?
Where must the condition live so that both branches are explicit, and what does the original function look like at that point?
Why did per-step suite runs catch this on step 1 and not at the end, if you had squashed the steps?
Write the assertion that would have caught it without the corpus — what property of the result is violated?

Reflection questions¶

Which principle was violated most expensively in the original, and how much did the line count understate that cost?
The three extractions created three seams. Which one is worth its cost, and which one would you skip for a program half this size?
The type string is now read in one place. What did that buy, and what does it cost for a type whose name is not known until runtime?
Your suite is a program with assertions, not a test framework. Where is that weaker, and what would you add first?
Adding a type now touches one file — is that always better? Give a case where a single edited function is the right design.
What did you decide not to refactor, and what is the risk of leaving it?

Completion checklist¶

[ ] It builds from a documented command with -Wall -Wextra -Werror and no warnings
[ ] before/ is preserved untouched, and the suite was captured from it
[ ] The suite has at least 25 cases and covers every boundary named in the brief
[ ] Every refactoring step is logged with its suite result
[ ] The suite is byte-identical before and after every step
[ ] No type-string branch remains in the calculation path — searched for and recorded
[ ] Pay, deductions and presentation do not know about each other
[ ] The two new types and the fourth location were actually attempted, with diffs recorded
[ ] refactor-notes.md lists violation → change → what still violates, for all five principles
[ ] The cost of the abstractions is stated, with one thing that got harder
[ ] All fourteen test cases run, with output recorded
[ ] The debugging challenge was explained before being fixed