optimize(refactor): add worked examples + counter-example to refactorer.md (d5 target)
This commit is contained in:
parent
e87b628ba8
commit
22bee3aeaa
@ -78,3 +78,93 @@ VIOLATIONS NOT FIXED (justified):
|
||||
|
||||
TESTS: ✅ passing / ❌ failures detected
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## WORKED EXAMPLES
|
||||
|
||||
### Example 1 — split oversized function
|
||||
|
||||
Before (32 logic lines, 6 locals → violates limits):
|
||||
|
||||
```ts
|
||||
function processOrder(o: Order) {
|
||||
const subtotal = o.items.reduce((s, i) => s + i.price * i.qty, 0);
|
||||
const discountRate = o.coupon ? lookupCoupon(o.coupon).rate : 0;
|
||||
const discount = subtotal * discountRate;
|
||||
const taxBase = subtotal - discount;
|
||||
const taxRate = o.region === "EU" ? 0.20 : 0.0875;
|
||||
const tax = taxBase * taxRate;
|
||||
const shippingCost = o.shipping === "express" ? 12 : 5;
|
||||
const total = taxBase + tax + shippingCost;
|
||||
// ...persist + email...
|
||||
saveOrder({ ...o, total });
|
||||
emailReceipt(o.email, total);
|
||||
return total;
|
||||
}
|
||||
```
|
||||
|
||||
After (each helper ≤ 25 lines, ≤ 5 locals, same external behavior):
|
||||
|
||||
```ts
|
||||
function computeSubtotal(items: Item[]): number { /* ... */ }
|
||||
function applyDiscount(subtotal: number, coupon?: string): number { /* ... */ }
|
||||
function computeTax(base: number, region: string): number { /* ... */ }
|
||||
function computeShipping(mode: string): number { /* ... */ }
|
||||
|
||||
function processOrder(o: Order): number {
|
||||
const subtotal = computeSubtotal(o.items);
|
||||
const taxBase = subtotal - applyDiscount(subtotal, o.coupon);
|
||||
const total = taxBase + computeTax(taxBase, o.region) + computeShipping(o.shipping);
|
||||
saveOrder({ ...o, total });
|
||||
emailReceipt(o.email, total);
|
||||
return total;
|
||||
}
|
||||
```
|
||||
|
||||
Tags applied: `[NORM] processOrder: 32 lines → split into 4 helpers`, `[NORM] processOrder: 6 locals → 3`.
|
||||
|
||||
### Example 2 — rename + extract guard
|
||||
|
||||
Before:
|
||||
|
||||
```py
|
||||
def h(d, u):
|
||||
if d.get("active") and u in d.get("admins", []):
|
||||
return d["data"]
|
||||
return None
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```py
|
||||
def get_data_if_admin(doc: dict, user: str) -> dict | None:
|
||||
if not _is_active_admin(doc, user):
|
||||
return None
|
||||
return doc["data"]
|
||||
|
||||
def _is_active_admin(doc: dict, user: str) -> bool:
|
||||
return doc.get("active") and user in doc.get("admins", [])
|
||||
```
|
||||
|
||||
Tags: `[NORM] h → get_data_if_admin (explicit name)`, `[QUALITY] guard extracted`.
|
||||
|
||||
### Counter-example — DO NOT refactor
|
||||
|
||||
Disguised business logic change (REJECT):
|
||||
|
||||
```ts
|
||||
// "refactor": collapse two branches → wrong, the order matters for audit log
|
||||
function charge(o: Order) {
|
||||
if (o.region === "EU") logVat(o); // must run BEFORE charge
|
||||
return chargeCard(o);
|
||||
}
|
||||
// proposed "cleaner" version:
|
||||
function charge(o: Order) {
|
||||
const result = chargeCard(o);
|
||||
if (o.region === "EU") logVat(o); // ← ORDER CHANGED, this is a behavior change
|
||||
return result;
|
||||
}
|
||||
```
|
||||
|
||||
Rule: if the diff changes ordering, side-effect timing, error visibility, or return-value semantics → it is NOT a refactor. Stop, report under `VIOLATIONS NOT FIXED` with reason "behavior change", and suggest opening a separate task.
|
||||
|
||||
Loading…
Reference in New Issue
Block a user