Code Structure

Code Structure

code-hygiene deletes the liability; this skill shapes the units that remain. Four owners, one test:

• hygiene = DELETE what git or a library already owns -- dead code, a name that lies or says nothing (code-hygiene), a restating comment (comment-hygiene). A rule that says delete X is one of the two hygienes'.
• code-structure = SHAPE THE UNITS -- how the code is decomposed, how units couple, how an interface reads, how a failure reaches a caller, how data carries its own invariants. Everything here assumes the code should exist; the question is what shape its units and boundaries take.
• readable-code = READ -- how a single body reads line by line: control-flow shape, naming across a set, reading order and working set. Anything about how one body reads rather than how the units are carved is readable-code's.
• pythonica = Python mechanics -- a rule that names a Python construct (dataclass, NewType, a context manager, match) is pythonica's; keep this skill language-agnostic.

Read code-hygiene and comment-hygiene first: shaping code that should have been deleted is wasted work. When a fix here would be self-documenting code (a revealing name, an extracted step, an explaining variable), that is exactly what comment-hygiene's "prefer self-documenting code over a comment" meta-rule points to -- that skill removes the comment, this skill supplies the structure. Where a smell reads as "there is redundant knowledge here", check the boundary: identical text in two places is code-hygiene's duplication rule; the same decision in two unlike forms is this skill's (DRY-as-knowledge, below).

Every rule below is a recognizable smell (something a reader or author can actually spot) -> the principle it violates -> a fix direction. No line, parameter, or nesting-depth count gates any rule here: that false precision is the size-fallacy this skill exists to replace. (Illustrative counts like "a two-line loop index" still appear -- they describe an example, they do not set a threshold.)

Decomposition and module depth

The stance (resolving the size tension). The oldest fight in this space is Martin ("small functions, extract till you drop, one thing per unit") versus Ousterhout ("deep modules, depth over count, beware classitis and shallow conjoined methods"). This skill takes a side. Decompose on responsibility and cohesion seams, never on size -- size is not the defect, doing-two-things is. Prefer depth (a narrow interface hiding real work) over a pile of shallow one-line methods, because every unit you add is an interface the reader must learn, paid forever. And stop extracting the moment the pieces are more coupled apart than together: if you must read the extracted helper to understand its caller, it was one thought -- leave it whole. The rest of this family is that stance applied.

One caveat scopes the whole family: a name introduced within a body -- an explaining variable or a local named predicate -- costs no caller an interface and is never what "too many units" warns against; only an extracted unit a caller must learn (a method, a module) is counted by the depth-over-count rule. So "name the subexpression" (in readable-code) and "beware the shallow swarm" never actually collide.

• Cannot name it without "and" -- SMELL: to name the unit you reach for a conjunction (and, or, then) or a vague verb (handle, process, manage, do), and the honest name promises less than the body delivers, so a reader must scan the whole body to learn what a call does. PRINCIPLE: a unit does one nameable thing; the conjunction you reached for is the seam between two responsibilities. FIX: split where the and falls so each piece earns a conjunction-free name -- but stop when the halves are more coupled apart than together.
• Interface almost as wide as the implementation -- SMELL: you read the signature and still must read the body to call it safely; the parameters, exceptions, and ordering rules a caller must learn nearly equal the code the unit saves (a parse(text, mode, strict, encoding, on_error) that saves the caller five lines but forces them to learn five knobs). PRINCIPLE: modules should be deep -- a narrow interface concealing substantial work; the interface is a cost every caller pays, the implementation is paid once. Depth, not method or class count, measures good decomposition. FIX: widen what the module does behind the same narrow interface, or fold a shallow layer into its neighbour. Judge a split by whether the interfaces got simpler, not whether the pieces got smaller.
• A swarm of shallow one-liners -- SMELL: to follow one operation you open file after file, ping-ponging across single-use helpers that mean nothing except in the order the original caller invoked them (_step1_load, _step2_massage, _step3_emit, each called once, each reaching into the last one's state). Each cut added an interface while hiding almost nothing. PRINCIPLE: more, smaller components is not automatically better; a cut that leaves two pieces you cannot understand apart added cost with no benefit. FIX: merge units that only ever appear together and reach into each other's state back into one deeper unit. This is the direct counter to "extract till you drop". (Contrast reordering: a single-caller helper that is a genuine, independently nameable unit is merely misplaced, not shallow -- move it, do not merge it. See the "Scattered pieces of one thought" rule in readable-code.)
• Pass-through / middle man -- SMELL: a unit forwards its arguments to another with nearly the same signature, or a class exists only to hold one method that delegates elsewhere -- def save(u): return self._store.save(u). A thin wrapper that adds a name and a hop but makes no decision and holds no data. PRINCIPLE: indirection must earn its keep by hiding real work; a pure pass-through is depth-zero -- full interface cost, no hidden substance. FIX: inline it into its caller, or let clients reach the real object directly; re-extract only when a layer genuinely adds an abstraction. (Boundary: unlike hygiene's orphaned abstraction -- a wrapper with no live caller, which is dead code to delete -- a middle man has real callers; it is not a liability to remove but depth-zero indirection to reshape.)
• Altitude jumps mid-body -- SMELL: reading top to bottom, a call to charge_customer() sits a line or two from raw index arithmetic; you keep switching between what the code intends and how a detail works, inside one body. PRINCIPLE: a function reads best at one altitude, as a paragraph of intent with the next level of detail one call down (the step-down rule) -- but altitude is a proxy; the real target is a responsibility seam, not uniform elevation. FIX: treat an altitude jump as a prompt to look for a hidden seam -- extract the low-level mechanics into a named helper only when that block is an independently nameable operation; equalising altitude is not itself a reason to cut. Where no such seam exists, leave the computation whole rather than shatter it into one-liners.

Cohesion and change locality

One reason to change. Grouping is a cohesion property, never a size limit.

• Same file in unrelated pull requests -- SMELL: the file lands in a pricing tweak one week and an export-format tweak the next, and the two edits sit in regions that never touch. PRINCIPLE: a module should have one axis of change; two unrelated reasons to edit it means two responsibilities cohabiting. FIX: split along the axis of change so each module answers to one kind of change -- gather what changes together, separate what changes for different reasons.
• One change, scattered edits -- SMELL: adding a payment method or renaming a status forces the same small edit across many files that share no text, and it is easy to miss one, so the change survives only as a map you carry in your head. PRINCIPLE: what changes together should live together; one concept smeared across modules turns one logical change into many physical ones with silent gaps. FIX: gather the scattered pieces behind one owner, or invert the dependency so the knowledge lives where it is used.
• A design decision known in two places (information leakage) -- SMELL: two modules that never share a line of text nonetheless both encode the same design decision -- a wire format, an on-disk layout, a status vocabulary, a fixed sequence of protocol steps -- so a change to that decision must be made in both, and no compiler links them. PRINCIPLE: a single design decision should be known to exactly one module; a decision reflected in several is leaked, and leakage is a leading cause of shallow, entangled modules. FIX: give the decision one owning module and route the others through it. Distinguish from DRY-as-knowledge below (the same decision copied in unlike text a search could in principle find) and from temporal decomposition below (whose reader-lives-with-writer cure is leakage removed): here the tell is two modules that must both know one fact, with nothing duplicated to grep for.
• Modules named after execution phases -- SMELL: Reader then Processor then Writer, or Setup / Run / Teardown, each holding a slice of the same subject, so any change to that subject touches all of them. PRINCIPLE: decompose by knowledge and responsibility, not by order of execution; temporal order is a runtime fact, not a design boundary. FIX: regroup so each module owns one subject end to end (the code that reads a format lives with the code that writes it). When tempted to split by time-order, ask what distinct knowledge each part would hide -- if none, do not split there.
• One decision, many unlike forms (DRY as knowledge) -- SMELL: to change one rule you edit several places that look nothing alike -- a limit set in a schema and re-checked in a handler, a tax rate written in code and again in a calculation elsewhere -- so a text search will not find them all and one silently keeps the old value. PRINCIPLE: DRY is about knowledge, not text; every decision should have one authoritative representation. Because these copies share no text, hygiene's collapse-the-duplicate rule never flags them -- this is why the rule lives here. FIX: give the decision one owner and derive the rest (generate, compute, reference). Ask: if this rule changed, how many places would I have to find? The answer should be one.
• The flag-ridden shared helper (the wrong abstraction) -- SMELL: a helper once extracted from duplication now sprouts boolean parameters and if-branches, each added by a caller whose need diverged; to read it for your case you mentally strip the branches that do not apply. PRINCIPLE: a little duplication is cheaper than the wrong abstraction -- two things identical today but changing for different reasons are not one piece of knowledge, and merging them couples independent decisions. FIX: inline the helper back into its callers, then re-extract only the parts genuinely identical and changing for the same reason. Decouple in preference to de-duplicating; unify only what encodes one decision.

Coupling and dependency direction

How units bind to each other, and which way the arrows point.

• Train wreck / message chain -- SMELL: a call walks through objects the caller should not know about -- order.getCustomer().getAddress().getZip() -- so it depends on the internal shape of things two and three hops away. PRINCIPLE: talk only to your immediate collaborators; reaching through a path welds you to internals you never asked for. The smell is reaching through, not a dot count. FIX: give the nearest collaborator a method that returns the end result (order.shipping_zip()) and hide the delegate. Caveat: a chain over a behavior-free data structure (a plain DTO) is not a Demeter violation -- the law is about behavior-rich objects.
• Feature envy -- SMELL: a function keeps pulling three fields off order to compute something while barely touching the object it lives on; its center of gravity is elsewhere. PRINCIPLE: behavior belongs with the data it operates on; splitting the two raises coupling and scatters the next change across both. FIX: move the method to the class whose data it uses, so the caller tells the object what to do instead of interrogating its fields. If only part envies, extract that part first, then move it.
• A local change that reaches into an unrelated concern (lost orthogonality) -- SMELL: a change you expected to be self-contained -- adding a report column, swapping the cache, retiming a job -- forces an edit in a concern that has nothing to do with it, because the two were wired together with no reason to be. PRINCIPLE: independent concerns should vary independently; when two things are orthogonal, a change to one leaves the other untouched. FIX: sever the incidental link -- separate the axes, invert or drop the dependency -- so each concern can move on its own. Distinct from feature envy (behaviour sitting on the wrong data) and from depend-toward-stability below (arrow direction): orthogonality is about two things that should not touch at all.
• Invisible agreement across a distance -- SMELL: two places in different modules must agree on something unstated -- the order of positional arguments, the meaning of a magic return code, a required call sequence -- so changing one silently breaks the other. PRINCIPLE: the stronger the coupling (position, meaning, execution order), the more it must be kept local; strong coupling across a distance is the costly kind. FIX: weaken the agreement (name the positional arguments so position stops carrying meaning, give a magic return code a named result type) or pull the coupled parties into one unit so the agreement is local and visible. (A bare number's meaning is hygiene's magic-value rule; here the defect is the locality -- an agreement stretched across a distance -- not the literal itself.)
• Stable policy names a volatile detail -- SMELL: a module that embodies durable policy references, by name, a thing that churns underneath it -- a specific pricing table, an experiment flag, a concrete class whose internals change -- so every change to the volatile detail ripples up into the stable policy, and you can trace the blast radius by following the imports. PRINCIPLE: depend in the direction of stability and abstraction; policy should not name the details that change beneath it. FIX: invert the reference so the volatile detail implements an interface the stable module owns -- then its churn stops rippling out. (This is the dependency-direction reading. The sibling case where the volatile detail is an external library named across many modules is "An external type named everywhere" below -- same wrapping move, but that rule owns confining a boundary to one seam, this one owns which way the arrow points. Scope: unit-to-unit dependency reading, not large-scale layering.)

Interface and contract design

The interface is everything a caller must learn to use the module; keep it simpler than the work it fronts (why that matters is the depth resolution above).

• Complexity pushed onto the caller -- SMELL: callers repeatedly supply what the module already knows (a buffer size, retry count, cache policy no caller can choose better than the author), or special-case the same edge at every site, or dispatch on a raw error state the module could have settled. The identical boilerplate reappears at every call site. PRINCIPLE: a value the module can compute internally should not be a parameter; a decision should not be pushed onto callers who know less than you do. FIX: pull complexity downward -- default or auto-tune the value, resolve the edge internally, return a directly usable result. Expose a knob only where callers genuinely hold context the module lacks.
• **A c

…

Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

• Author: Jartan-LLC
• Source: Jartan-LLC/grimoire
• License: MIT

Install and usage instructions live in the source repository linked above.