Five Discount Rows Are the Refactor Gate
Current discount behavior should stay pinned before any formula moves. A wide edit can change totals while tests still look green. Five recorded rows are enough to stop that silent shift.
The method is a characterization gate, then one extract. It does not fix known rounding bugs in the same change. A bug fix is a later commit with its own rows.
Why a wide cleanup fails
Messy discount code often mixes math, environment reads, and prints. A cleanup that touches all three can change money output. Reviewers then argue about style instead of totals.
Integer floor division is the hazard in this sample. A rate of fifteen on one hundred ninety-nine cents yields twenty-nine. A later half-up rule would be a different commit.
That one-cent gap is enough to hide inside a refactor. The gate records the old cent, not the preferred cent. Preference belongs in a separate reviewed change later.
The only function this pass may touch
The sample below is a proposal, not a production billing module. It reads two environment variables and mutates the order dict. It also prints a discount line through an injected sink.
import os
def apply_discount(order, sink=print):
rate = os.environ.get("DISCOUNT_RATE", "10")
cap = os.environ.get("DISCOUNT_CAP", "50")
subtotal = order["subtotal"]
raw = subtotal * int(rate) // 100
discount = raw if raw < int(cap) else int(cap)
order["discount"] = discount
order["total"] = subtotal - discount
sink(f"discount={discount}")
return order["total"]
Three side effects sit beside the formula itself. The order dict gains discount and total keys. The sink receives one formatted discount line each call.
The process environment supplies both rate and cap. Moving all three at once is not the smallest safe change. The smallest change lifts only the arithmetic block.
Environment reads, mutation, and printing stay in the wrapper. Callers keep the same function name and the same arguments. That boundary is the whole point of this pass.
Rows that hold the old cents
Each row fixes subtotal, rate, cap, discount, total, and the printed line. A missing rate or cap means the default string is in force. Negative subtotal is included because the current code allows it.
| Row | Subtotal | Rate | Cap | Discount | Total | Line |
|---|---|---|---|---|---|---|
| 1 | 199 | 15 | 50 | 29 | 170 | discount=29 |
| 2 | 1000 | 15 | 50 | 50 | 950 | discount=50 |
| 3 | 80 | default | default | 8 | 72 | discount=8 |
| 4 | 0 | 10 | 50 | 0 | 0 | discount=0 |
| 5 | -100 | 10 | 50 | -10 | -90 | discount=-10 |
Row one uses floor division on one hundred ninety-nine cents. That subtotal times fifteen is two thousand nine hundred eighty-five. Integer division by one hundred then yields twenty-nine.
Twenty-nine is under the cap, so the cap does not apply. The returned total is one hundred seventy cents. The printed line must match that same discount.
Row two exceeds the cap on a one thousand cent subtotal. One thousand times fifteen divided by one hundred is one hundred fifty. The comparison uses less-than, so one hundred fifty becomes fifty.
The pinned total is nine hundred fifty cents. A test that checks only this row can still pass. A small formula drift can hide behind the cap.
Row one is what catches the one-cent case. Row three checks the default rate and the default cap. Eighty times ten divided by one hundred is eight.
Eight is under the default cap of fifty. The printed line is still emitted for that row. Defaults are behavior, not comments beside the code.
Editing the default strings is a behavior change. Row five is a preserved quirk, not a billing rule. Negative ten is less than fifty, so the cap does not clamp it.
Do not treat that row as a product requirement. Python floor division makes negative one hundred times ten land on negative ten. That result is pinned only so the extract stays honest.
1. Store the rows as data
Keep the expected numbers together in one list. Loop the list so a new row is data, not a new test name. Use monkeypatch so environment edits do not leak.
from discount_legacy import apply_discount
ROWS = [
{
"subtotal": 199,
"rate": "15",
"cap": "50",
"discount": 29,
"total": 170,
"line": "discount=29",
},
{
"subtotal": 1000,
"rate": "15",
"cap": "50",
"discount": 50,
"total": 950,
"line": "discount=50",
},
{
"subtotal": 80,
"rate": None,
"cap": None,
"discount": 8,
"total": 72,
"line": "discount=8",
},
{
"subtotal": 0,
"rate": "10",
"cap": "50",
"discount": 0,
"total": 0,
"line": "discount=0",
},
{
"subtotal": -100,
"rate": "10",
"cap": "50",
"discount": -10,
"total": -90,
"line": "discount=-10",
},
]
def _apply_env(monkeypatch, row):
if row["rate"] is None:
monkeypatch.delenv("DISCOUNT_RATE", raising=False)
else:
monkeypatch.setenv("DISCOUNT_RATE", row["rate"])
if row["cap"] is None:
monkeypatch.delenv("DISCOUNT_CAP", raising=False)
else:
monkeypatch.setenv("DISCOUNT_CAP", row["cap"])
def test_discount_rows_hold(monkeypatch):
for row in ROWS:
_apply_env(monkeypatch, row)
lines = []
order = {"subtotal": row["subtotal"]}
total = apply_discount(order, sink=lines.append)
assert total == row["total"]
assert order["discount"] == row["discount"]
assert order["total"] == row["total"]
assert lines == [row["line"]]
Run the gate before any edit to the formula. The commands assume pytest is installed in the active environment. A failing import means the module path is wrong.
python -m pytest tests/test_discount_rows.py -q
A green run means the five rows match this checkout. A red run means the table is wrong for this tree. Fix the table before you extract any formula.
2. Force a known failure
Add one cent to the floor result in a scratch copy. Do not commit that probe to the branch. The point is to prove the rows can fail.
raw = subtotal * int(rate) // 100 + 1
Row one then expects thirty instead of twenty-nine. The pinned assertion fails on that changed total. The failure comes from integer arithmetic alone here.
Row two stays capped, so it may still pass. Row one is the probe that must fail. A cap-only test would miss this one-cent shift.
Restore the floor operator after the probe ends. The characterization file should be green again afterward. If it stays red, the probe leaked into the branch.
Check the diff before you continue. The scratch edit should be gone from the diff. A leftover plus-one is already a behavior change.
git diff -- discount_legacy.py
Do not extract on top of a dirty probe. A clean diff is the entry ticket for step three. The characterization file should still be the only new test.
3. Lift one pure function
Create one pure function with the same floor rule. Keep the same less-than cap comparison inside it. Leave environment access inside the existing public wrapper.
Leave the print line inside that same wrapper. The public wrapper signature stays the same after extract. Callers that pass a sink still work unchanged.
def discount_amount(subtotal, rate, cap):
raw = int(subtotal) * int(rate) // 100
limit = int(cap)
if raw < limit:
return raw
return limit
def apply_discount(order, sink=print):
rate = os.environ.get("DISCOUNT_RATE", "10")
cap = os.environ.get("DISCOUNT_CAP", "50")
discount = discount_amount(order["subtotal"], rate, cap)
order["discount"] = discount
order["total"] = order["subtotal"] - discount
sink(f"discount={discount}")
return order["total"]
Callers that rely on dict mutation still work. Re-run the same test file after the extract. Do not add new assertions in this commit.
python -m pytest tests/test_discount_rows.py -q
git diff --stat -- discount_legacy.py
A passing run with a small diff is the stop condition. The diff should show one new function and one call site. It should not show a rounding policy change.
4. Decide with a table, not a debate
Stop when the rows pass and the diff is only the extract. Continue only if a listed signal appears below. Each listed signal maps to one action only.
| Signal | Action |
|---|---|
| Rows fail after the extract | Revert the extract and compare operators |
| Diff edits the print format | Move that edit to a later commit |
| Diff edits default rate or cap | Revert it; row three pins those defaults |
| Diff replaces floor division | Reject it; that is a behavior change |
| Rows pass and the diff is one function | Stop and file rounding as a separate ticket |
The table is the review checklist for this extract. It is shorter than a style debate in review. It also names the forbidden second change clearly.
5. Draft assertions, then delete extras
MonkeyCode free model access can draft that assertion list. Disclosure: This article was prepared as part of MonkeyCode's product outreach. Free model access does not choose the correct money rule.
You still own every expected total in the table. Paste the five-row table into the model prompt. Ask for a pytest loop that calls the existing wrapper.
Require the draft to import the current module path. Reject any assertion that changes twenty-nine to thirty. If the draft adds rows you did not write, delete those rows.
Run that draft on the free server option first. Keep the shared laptop tree untouched until the rows pass. The server is a clean checkout for this gate.
It is not a claim about hardware or uptime. Read the failure line before you edit production code. A mismatch on row one means the formula moved.
A mismatch on the printed line means the sink format moved. Those two mismatches belong in two different commits. A green draft is not a fairness proof.
python -m pytest tests/test_discount_rows.py -q --tb=short
6. Lock the pure function in a second commit
Add a direct call only after the wrapper rows pass. This check guards the extracted function from a later wrapper edit. It is optional, and it is not part of the extract commit.
from discount_legacy import discount_amount
def test_discount_amount_matches_row_one():
assert discount_amount(199, "15", "50") == 29
assert discount_amount(1000, "15", "50") == 50
Keep this file separate if review wants a small commit. The first commit proves the wrapper did not move. The second commit names the pure function as a stable seam.
Limits of the five rows
Five rows do not cover a non-integer rate string. Calling int on the text ten point five raises ValueError. That path stays unpinned until you add a row for it.
The tests use monkeypatch inside the test process. They do not model concurrent writers to the process environment. Do not cite this file as a concurrency proof.
The negative row preserves a quirk so the extract stays honest. Shipping that quirk to customers is a product decision. This gate does not make that product decision.
The plus-one probe is a local failure check only. It is not a benchmark against other tools. No timing numbers are claimed in this note.
No model quality score is claimed in this note. Free server access is an availability option for this workflow. Free model access is the other availability option.
They are not a promise of quota or duration. They also are not a named model build. If either option is unavailable, run the same pytest file locally.
Who should not use this gate
Skip it when this week's goal is to correct the money rule. Pinning twenty-nine would only delay the required fix. Write the new rule, then replace the row on purpose.
Skip it when the repo has no test runner. An extract without a failing probe is a guess. Guessing is how one-cent gaps land in review.
Skip it when the printed line is an unstable debug string. Pin the returned total and the dict fields instead. A noisy line will fail for reasons unrelated to money.
Skip it when subtotal is not an integer count of cents. This sample assumes integer cents and integer percent text. Decimal currency needs a different row set entirely.
It also needs a different pure function body. Do not reuse these five integers for decimal invoices. The gate would pin the wrong money unit.
Close this commit
Record the five discount rows before any move. Probe those rows with a plus-one formula next. Restore the floor rule after that probe ends.
Then extract one pure function and nothing else. Re-run the same file and stop there. Open a follow-up only for the rounding change you actually want.
That follow-up should show row one moving on purpose. The characterization commit should not contain that move. Use a free server checkout when the shared laptop tree cannot take the probe.
Top comments (1)
tr.ee/dev-to