| name | refactor |
| description | Use this skill when extracting helpers, renaming functions, replacing magic numbers, or otherwise restructuring code without changing behavior. Covers extract method/namespace, rename, magic-number-to-constant, and the discipline of rewiring callers immediately. |
Refactoring Lessons
When This Skill Applies
Use this skill whenever you are restructuring code without changing behavior — extracting helpers, renaming functions, replacing magic numbers, splitting a god namespace, or moving logic to a more cohesive home.
Shared code extraction is not finished until callers use it
A refactoring is not "new code plus new tests living beside the old code."
It is complete only when existing production callers have been moved to the extracted code and duplication has been removed.
If an extracted namespace, class, or helper has no production callers yet, stop and treat that as a warning sign: the work is incomplete and likely drifting into side-car development.
Preserve behavior before improving structure
Start from green tests.
Make the smallest possible extraction seam, immediately rewire an existing caller to use it, and run the relevant tests after each tiny step.
Only then continue removing duplication.
The sequence is:
- Start green.
- Extract one small helper.
- Rewire real production code to use it.
- Run tests.
- Remove the now-redundant duplication.
- Run tests again.
That is refactoring. Anything else risks adding unused, tested code without changing the design that actually runs.
Do not change semantics during extraction
Behavioral drift during extraction is easy to miss, especially when the new code is not yet wired into production.
When you extract a function, you are promising that it does exactly what the
original inline code did — nothing more, nothing less. The most common drift:
adding a guard, changing a default, or broadening a nil case "while you're in
there." Each of those is a behavior change disguised as cleanup.
Symptoms:
- The extracted version returns a value where the original returned nil.
- The extracted version accepts a broader input range than callers ever provide.
- A conditional that was implicit in the call site becomes explicit in the
helper — but with a different default branch.
If the extraction is wired into production immediately, these drifts surface
as test failures right away. If the extraction lives beside the old code with
its own isolated tests, the drift hides until someone finally switches callers
over — and by then the divergence is baked in.
Extract Method/Function
When a piece of logic or setup repeats, extract the smallest useful function, name it from the caller's point of view, and immediately switch the callers over.
Concrete steps followed here:
- Spot repeated test setup.
rpc_spec.clj built the same BufferedReader/StringReader combination more than once.
- Extract a tiny helper.
- Added
(defn reader-for [line] (BufferedReader. (StringReader. line))).
- Rewire all existing callers.
- Replace repeated reader construction with
reader-for.
- Run tests.
- Verify behavior is unchanged.
- Keep going only if more duplication remains.
- The same pattern was applied to JSON-RPC data construction.
- Extract small protocol helpers.
- Added
jrpc/request-line, jrpc/request, and jrpc/result.
- Rewire specs to use the extracted helpers.
- Literal request/result construction moved to
acp.jsonrpc helpers.
- Run tests again.
- Confirm the extraction changed structure, not behavior.
Why this worked:
- The extracted functions were small and specific.
- Callers were updated immediately, so duplication actually disappeared.
- Each step was testable in isolation.
This is the part people skip: extraction is not finished when the helper exists. It is finished when the old call sites are gone and duplication is removed.
Extract Class/Namespace
"Extract Class" is the traditional refactoring name.
In Clojure, the equivalent move is usually extracting a cohesive namespace.
The goal is the same: move a tightly related set of behavior and data-shaping responsibilities out of an overgrown module and into a focused unit with a clear purpose.
Concrete example:
acp.jsonrpc became the natural home for JSON-RPC request/response construction and protocol constants.
- The extraction should group related behavior, not create a vague helper bucket.
- If the new namespace starts sounding like
util, common, or shared, the design is probably hanging crooked.
How to do it safely:
- Start with behavior already covered by tests.
- Identify one cohesive responsibility inside the existing namespace.
- Extract only the functions and constants that belong together.
- Rewire production callers immediately to use the new namespace.
- Run the relevant specs and features.
- Remove the old duplicated logic.
- Run tests again.
What counts as done:
- production callers use the extracted namespace,
- duplicated logic in the old location is removed,
- behavior is unchanged,
- the new namespace has a crisp responsibility.
What does not count:
- adding a new namespace beside the old code without rewiring callers,
- leaving duplicate protocol-building logic in both places,
- dumping unrelated helpers into the extracted namespace just because they are "shared."
This is why acp.jsonrpc is a good example. It is not a grab bag. It is a protocol-focused namespace with a narrow reason to change.
Replace Magic Number with Constant
If a numeric literal carries protocol or domain meaning, give it a name and use that name everywhere.
Concrete example:
- JSON-RPC parse error code
-32700 appeared in production code and specs.
- That value is protocol knowledge, not local implementation detail.
- Extract
PARSE_ERROR into acp.jsonrpc.
- Replace every use of
-32700 with jrpc/PARSE_ERROR.
This keeps the meaning centralized, removes repeated knowledge, and makes future protocol changes less error-prone.
Rename Method/Function
Fowler's first edition called this "Rename Method." The second edition folds it into "Change Function Declaration" (aliases: Rename Function, Rename Method, Add Parameter, Remove Parameter, Change Signature).
Why
Names are the most important thing in code. When you understand what a function does better than its name suggests, rename it immediately. A good name eliminates the need to read the body. Getting names wrong the first time is normal — the refactoring exists because understanding deepens over time.
Simple Mechanics
Use this when you can update the declaration and all callers in one pass:
- Start green.
- Change the function declaration to the new name.
- Find all callers. Update each one.
- Run tests.
If you need to change name and parameters, do them as separate steps.
Migration Mechanics
Use this when there are many callers, the function is part of a protocol/interface, or you want a gradual rollout:
- Start green.
- Create the new function. If the name is the same as the old one, use a temporary name.
- In Clojure: prefix protocol methods with
-, add public forwarding functions with the desired name and arity.
- Delegate the old function to the new one (or vice versa).
- Run tests.
- Migrate callers one by one. Run tests after each.
- When no callers of the old function remain, remove it.
- If you used a temporary name, rename to the final name now.
- Run tests.
For published APIs or protocol methods with external consumers, you can pause after step 4 and leave the old function as a deprecated delegate while callers migrate.
Practical rule
When extracting shared logic:
- do not leave the old implementation in place while adding a mirrored new one,
- do not write isolated tests for code that production does not use yet unless they support an immediate rewiring step,
- do not commit until the extraction is wired through real callers, duplication is removed, and the test suite is green.
A fun little compiler-adjacent truth: dead code with tests still has negative design value. It proves something correct that the system does not actually do.