> ## Documentation Index > Fetch the complete documentation index at: https://lib.findy.co.jp/llms.txt > Use this file to discover all available pages before exploring further. # Refactoring > Change structure without changing behavior: separate structure from behavior changes, proceed in small reversible steps, and design code for deletion. ## Overview Refactoring is changing the internal structure of code without changing its observable behavior. Structure drifts as a codebase absorbs change after change, and refactoring is the ongoing work that restores [modifiability](/development/modifiability) — the property that keeps the next change cheap and safe. Refactoring is not a special project or a phase that waits for permission: it can happen at any time, and it is always in demand. What needs discipline is not *when* to refactor but *how* — in units that a reviewer can verify, in steps that can be undone, and toward a structure whose parts can eventually be deleted. This page covers three practices that provide that discipline — separating structure changes from behavior changes, proceeding in small reversible steps, and designing for deletion — and the role of test code that underpins all of them. ## Structure changes and behavior changes Every change to code belongs to one of two categories, and the two are verified in entirely different ways. | | Structure change | Behavior change | | ------------------ | ------------------------------------------------------------------------ | ------------------------------------- | | What changes | Names, boundaries, dependencies, duplication — how the code is organized | What the software observably does | | Who notices | Only developers reading the code | Users, API consumers, tests | | How it is verified | Existing tests pass, unchanged | Updated tests assert the new behavior | | Risk profile | Low, and mechanically checkable | Carries product risk by definition | The examples below use a common running example: a hook that fetches order history and shapes it for display. ```tsx Running example: fetching and shaping order history theme={null} // Fetch order history and shape it for display export const useOrderHistory = () => { const { data } = useOrders(); // Hide cancelled orders and sort newest first const orders = (data ?? []) .filter((order) => order.status !== "cancelled") .sort((a, b) => compareDesc(a.orderedAt, b.orderedAt)); return { orders }; }; ``` Which category a change to this hook belongs to is decided by one thing: whether the list content — the observable behavior — changes. In a structure change, only the code arrangement changes and the hook returns the same list, so existing tests pass unchanged. ```tsx Structure change: extract the shaping logic into a function theme={null} const selectOrders = (data: Order[] | undefined) => (data ?? []) .filter((order) => order.status !== "cancelled") .sort((a, b) => compareDesc(a.orderedAt, b.orderedAt)); export const useOrderHistory = () => { const { data } = useOrders(); return { orders: selectOrders(data) }; }; ``` In a behavior change, the list content changes, so the tests are updated to the new behavior. ```tsx Behavior change: stop excluding cancelled orders theme={null} export const useOrderHistory = () => { const { data } = useOrders(); // Drop the filter and keep only the newest-first sort const orders = (data ?? []) .sort((a, b) => compareDesc(a.orderedAt, b.orderedAt)); return { orders }; }; ``` Refactoring, by definition, covers only the first kind — the structure change. The moment a change alters what the software does, it is no longer a refactoring — it is a behavior change that deserves a behavior change's scrutiny. ### Do not mix the two in one pull request A structure change and a behavior change must not share a pull request. The reason is how each is reviewed. A structure-only pull request is verified with a single question: is the behavior identical? The evidence is mechanical — no test was modified, and every test passes. A reviewer can approve a large structural diff quickly, because the tests answer the hard question. A behavior-change pull request is reviewed against its intent: the diff includes updated tests, and the reviewer checks that the new assertions match the specification. A pull request that mixes a structure change and a behavior change destroys both verification strategies. The tests changed, so "tests unchanged and passing" no longer proves behavior is preserved — the reviewer must classify every line by hand, asking of each whether it was supposed to change behavior or not. The review becomes slower and less reliable at the same time. Using the earlier extraction as the example, the two treatments differ like this. First, the pattern where the pull request only extracts: the tests are untouched, and passing unchanged is itself the evidence that behavior is preserved. Showing cancelled orders ships as a follow-up behavior-change pull request with its updated tests. ```tsx ✅ Implementation: this pull request only extracts theme={null} const selectOrders = (data: Order[] | undefined) => (data ?? []) .filter((order) => order.status !== "cancelled") .sort((a, b) => compareDesc(a.orderedAt, b.orderedAt)); export const useOrderHistory = () => { const { data } = useOrders(); // Calling the same logic as before keeps the list identical return { orders: selectOrders(data) }; }; ``` ```tsx ✅ Test: passes unchanged theme={null} // The tests are untouched — passing unchanged is the evidence // that the observable behavior is preserved test("returns order history, newest first", () => { const { result } = renderHook(() => useOrderHistory()); // cancelled orders are excluded from the list expect(result.current.orders).toEqual([delivered2, delivered1]); }); ``` Mixing the extraction (structure) and the change to the visible orders (behavior) in one pull request instead rewrites the test expectations in the same diff: "tests pass unchanged" is lost as evidence, and the reviewer classifies every line by hand. ```tsx ❌ Implementation: structure and behavior share one diff theme={null} const selectOrders = (data: Order[] | undefined) => (data ?? []) // Alongside the extraction, the filter is dropped so // cancelled orders are now shown .sort((a, b) => compareDesc(a.orderedAt, b.orderedAt)); export const useOrderHistory = () => { const { data } = useOrders(); return { orders: selectOrders(data) }; }; ``` ```diff ❌ Test: expectations rewritten in the same diff theme={null} test("returns order history, newest first", () => { const { result } = renderHook(() => useOrderHistory()); - // cancelled orders are excluded from the list - expect(result.current.orders).toEqual([delivered2, delivered1]); + // cancelled orders now appear in the list + expect(result.current.orders).toEqual([delivered2, cancelled1, delivered1]); }); ``` The revert unit degrades the same way. A pull request should be a unit that can be reverted alone — see [Task Breakdown](/development/task-breakdown). Reverting a mixed pull request throws away a correct structure change to undo a wrong behavior change, or the reverse. When a reviewer requests a refactoring on a behavior-change pull request, it does not have to be fixed inside that pull request. Accepting the point and doing it as a follow-up structure-only pull request keeps both changes verifiable. ### The timing is a choice, not a rule Structure changes and behavior changes go into separate pull requests, but their order in time is a genuine choice. * **Structure first** — when the current structure makes the coming behavior change awkward, reshape the code first. The behavior change that follows becomes smaller and easier to review. * **Structure after** — when the behavior change is urgent or the structural problem only becomes visible while working, ship the behavior first and clean up immediately after, leaving the area ready for the next change. * **Not now** — when the code is rarely touched, the cost of restructuring may never pay back. Leaving it alone is a legitimate decision. A practical basis for the choice is how often the code changes: structure that many future changes will pass through pays back quickly, and structure nobody revisits pays back never. Delivering observable behavior early also reduces uncertainty — polishing structure before the behavior is validated optimizes code that may not survive. ## Tests define "behavior unchanged" Refactoring and test code are inseparable. "Existing tests pass unchanged" — the way a structure change is verified — only works when the tests genuinely assert the observable behavior. In other words, the claim that a refactoring preserves behavior is only as strong as the tests that assert the behavior. Two signals during a refactoring deserve attention: * **A test had to change.** Either the change was not actually structure-only, or the test asserts implementation details rather than observable behavior. Both cases need fixing — the first in the diff, the second in the test. See [Test Code](/development/testing). * **No test failed while the logic was temporarily broken.** If an intermediate mistake produced no failing test, the safety net has a hole in the area the refactoring touches. ## Small, reversible steps The cost of a change grows faster than its size: a larger diff takes disproportionately longer to review, conflicts with more concurrent work, and hides mistakes better. Refactoring therefore proceeds as a series of small steps, each of which keeps the system fully working. The rhythm of a step is fixed: make one structural move, verify that everything still passes, and commit. Renames, extractions, and moves each land as their own verified step. A step that turns out wrong is undone by reverting one small commit, not by untangling it from a day of other edits. The same rule applies at the pull request level. Merge small structure-only pull requests into the default branch continuously, rather than accumulating them on a long-lived refactoring branch. A long-lived branch collects conflicts with every concurrent change, and its eventual merge is exactly the large, risky diff the small steps were meant to avoid. Keep each pull request small enough that a reviewer finishes it in minutes. A restructuring too large for a single safe step is done as a **parallel change**: the new structure is built alongside the old, and traffic moves over incrementally. The three steps below walk through replacing a price formatter with a multi-currency one. Add the new structure next to the old one, with nothing using it yet. Behavior is untouched, so this step is trivially safe. ```tsx Expand: add the new function next to the old one theme={null} // The existing function is untouched export const formatPrice = (price: number) => `¥${price.toLocaleString()}`; // Add the new function; nothing uses it yet export const formatMoney = ({ amount, currency }: Money) => `${amount.toLocaleString()} ${currency}`; ``` Move callers from the old structure to the new one, a few at a time. Each migration is a small, independently revertible pull request, and the system works at every point in between. ```diff Migrate: move callers to the new function one by one theme={null} - {formatPrice(order.total)} + {formatMoney({ amount: order.total, currency: order.currency })} ``` When nothing uses the old structure, delete it. The refactoring is not finished until this step lands — a codebase carrying both structures is harder to change than before the refactoring started. ```diff Contract: delete the old function once nothing uses it theme={null} - export const formatPrice = (price: number) => - `¥${price.toLocaleString()}`; ``` The pattern works across ownership boundaries as well: renaming a field in an API response follows the same expand, migrate, contract sequence, with the provider and consumers moving in separate, individually safe steps. ## Designing for deletion Many refactorings end with a deletion: the contract step removes the old structure, an experiment that did not survive is removed, a feature flag whose rollout completed is retired — see [Release Strategy](/development/release-strategy). Deletion is where the classification of changes meets design: deleting code is easy, but deleting it **safely, quickly, and simply** is a property the code either was given when it was written or does not have — see [Modifiability](/development/modifiability). Code is easy to delete when its dependents can be enumerated mechanically and its parts live together. This is another way of saying the structure has high **cohesion** (code that changes together lives together) and low **coupling** (unrelated code does not depend on each other) — and the pair also indicates which direction a refactoring should move the structure. Cohesion has recognized levels of strength: the more essential the reason a module's parts are together, the stronger the cohesion. | Cohesion | Why the parts are together | Typical example | | ---------------------- | ----------------------------------- | ------------------------------------------------------------------------------ | | Functional (strongest) | They serve a single purpose | The UI, logic, and tests that make up one feature | | Sequential | One part's output feeds the next | A flow from fetch through transform to format | | Communicational | They operate on the same data | Operations reading and writing the same entity | | Procedural | They share only an execution order | Checks that run in sequence before a screen renders | | Temporal | They run at the same time | Initialization invoked together at startup | | Logical | They are the same kind of thing | A general-purpose function switching on a flag; utilities grouped only by kind | | Coincidental (weakest) | They happen to be in the same place | A miscellaneous `common` module | The priority is clear: aim for functional cohesion, accept sequential and communicational, and treat logical and coincidental cohesion as targets for decomposition. When the reason code is grouped shifts from "it changes together" to "it is the same kind" or "it happened to be there," that is the sign of weak cohesion. The following practices keep cohesion high and coupling low: * **Keep dependencies explicit and one-directional.** The question "what depends on this?" must have a mechanical answer — from types, imports, or module boundaries. Code whose dependents cannot be listed cannot be deleted with confidence. * **Group code by concept, not by kind.** When everything a feature needs lives in one subtree, deleting the feature is deleting the subtree. When it is scattered across shared layers, every deletion is an investigation. Places grouped only by kind — `utils`, `common` — are the typical form of logical or coincidental cohesion. * **Treat shared abstractions as deletion risks.** Every new caller of a shared helper makes that helper harder to remove or change. Extract shared code when duplication has proven itself a burden, not in anticipation. * **Delete dead code instead of keeping it.** Commented-out blocks, unused exports, and flags that never flip are not free: they mislead readers and dilute change detection. Version control already preserves everything — restoring deleted code is one revert away. Grouping by concept instead of by kind shows most clearly in the directory structure. ```text Grouped by kind vs. grouped by concept theme={null} # ❌ Grouped by kind: removing order history means # investigating every layer src/ components/OrderList.tsx hooks/useOrderHistory.ts utils/orderFormatters.ts # ✅ Grouped by concept: removing order history is # deleting one subtree src/ features/order-history/ OrderList.tsx useOrderHistory.ts formatters.ts ``` The forward-looking form of this practice is a question asked while writing new code: how will this be removed? A module that has an answer — its dependents are enumerable, its parts are together, nothing reaches into its internals — is cheap to delete, and for exactly the same reasons it is cheap to change. ## Related pages The classification of changes — separate, revise, remove — that refactoring keeps cheap. Split work into independently revertible units — the same principle that separates structure from behavior. The tests that make "behavior unchanged" a verifiable claim instead of a hope. Keep each pull request focused on one intent so review stays fast and reliable.