Method Decomposition

Overview

Methods exist to be read, not to be short. Optimise for a reader who has never seen the file. This skill is the full reference: the thresholds for the common case, the nuanced cases (when NOT to extract, stack-specific boundaries), and the sources to cite when justifying a decomposition decision.

Core principle (SLAP — Single Level of Abstraction Principle, Beck/Martin): a method does one thing at one level of abstraction. Cohesion over line count.

Fowler's heuristic (Refactoring, 2nd ed., p. 110): "the key is the semantic distance between the method name and the method body. If extracting improves clarity, do it, even if the name is longer than the code you have extracted." The corollary is also true: if it does NOT improve clarity, do not extract.

Workflow

When a method's shape or size is the question, work through these steps in order:

1. Locate the decision point. Identify the specific method you are writing, expanding, or being asked to split — this guidance applies per method, not per file.
2. Measure before judging (Java, on demand). If complexity is the real question and the method nests lambdas/streams/switches, run scripts/measure-complexity.sh <file.java> for real cognitive/cyclomatic numbers instead of guessing. Skip for Kotlin/TS or when PMD is absent — estimate by reading.
3. Compare against the limits. Check the method against the Concrete limits table (length, cognitive/cyclomatic complexity, nesting, parameters). Treat every breach as a smell to investigate, not an automatic failure.
4. Decide extract vs. inline. Apply When to EXTRACT and When NOT to extract. Extract only when the name encodes a distinct level of abstraction or domain rule; never to satisfy a line count.
5. Respect stack boundaries. Before moving code, apply the stack-specific notes — @Transactional self-invocation, WebFlux chain stages, @Service / repository / SQS layering — and never reorder I/O calls as a side effect of decomposition.
6. Self-check. Run the Self-check before submitting list on each method you changed; fix anything that fails the step-down / single-abstraction test.

Companion skill — java-flow-guideline: different altitude, not a rival. That skill decides where an artifact belongs (which layer/package) and how to build and test it; this skill decides how to shape a method's internals once it's placed. When scaffolding a new controller, consumer, service, or use-case that carries non-trivial logic, load both — they compose, neither overrides the other.

Concrete limits (defaults — deviate only with a comment explaining why)

These are smells, not laws. A coherent 45-line method that reads top-to-bottom is fine; a 25-line method that needs you to chase 6 helpers to understand it is not.

Measure, don't eyeball (on demand — Java)

Cognitive/cyclomatic complexity guessed from reading is unreliable on long methods with nested lambdas, streams, or switches. When complexity is the actual question for a Java method you are about to write, expand, or decompose, get the real number first — then apply the judgment above to it. Do not run this on every edit or as a gate; run it once, at the decision point.

Run the bundled script (it self-locates assets/pmd-ruleset.xml and skips cleanly if PMD is absent):

scripts/measure-complexity.sh <file.java>        # or no args → uncommitted + untracked .java files

Output is one line per method over threshold, e.g. Foo.java:105: CyclomaticComplexity: The method 'calculate(...)' has a cyclomatic complexity of 13. The ruleset reports cognitive complexity > 15 (S3776) and cyclomatic > 10 (S1541), matching the table above. A clean run (no output) means every method is under threshold — no measurement-driven extract is warranted.

The number informs, it does not command. A high cyclomatic count on a flat switch with no nesting is often fine to read (that is why the cognitive metric exists); use both numbers as one input to the "when to extract / when NOT to extract" judgment, never as an automatic trigger. If PMD is not installed (see compatibility) or the file is Kotlin/TS, skip this step and estimate by reading.

When to EXTRACT a method

Extract when any of these is true:

1. A block needs a leading comment to explain what it does — replace the comment with a method whose name IS the comment (Fowler's classic trigger).
2. The same (or near-identical) logic appears 2+ times (DRY; rule of three for non-trivial duplication).
3. The method mixes levels of abstraction (high-level orchestration interleaved with low-level string parsing) — violates SLAP.
4. The method exceeds any threshold above.
5. A block has a distinct testable behaviour otherwise tested only indirectly.
6. A boolean/conditional takes more than one mental pass to read → extract into boolean is…() / boolean has…() query method.

When NOT to extract (avoid "tiny methods" antipattern)

Do NOT extract when:

1. The extracted method would be called from exactly one place AND its name adds no information beyond the code (e.g. private int addOne(int x) { return x + 1; }).
2. Extraction forces a reader to jump to another method to follow a single linear thought (Ousterhout's "shallow module" critique in A Philosophy of Software Design — the cost of the interface exceeds the benefit).
3. The extracted code uses 5+ local variables from the caller — the parameter list itself becomes noise. Inline, introduce a parameter object, or refactor the parent first.
4. The block is trivial glue (1–3 lines, no branching, no domain concept worth naming).
5. Extraction crosses a transactional or reactive boundary in a way that changes semantics — see Stack-specific notes below.
6. You're only doing it to satisfy a line-count rule.

Uncle Bob's "functions should be 2–4 lines" stance (Clean Code, ch. 3) is rejected as a default. Martin himself clarified in johnousterhout/aposd-vs-clean-code on GitHub: "The 2-4 line functions that you referred to on page 34 were part of the Sparkle applet that Kent Beck and I wrote together in 1999 as an exercise for learning TDD… I claimed no final authority, nor even any absolute 'rightness'. They are offered for consideration."

Step-down rule (Clean Code ch. 4)

Public methods read like a table of contents; private methods sit below them at progressively lower abstraction levels. The first public method of the file should be readable as plain business prose with minimal jargon; details live further down.

Stack-specific notes (Java 25 / Spring Boot 3.x–4.x / WebFlux / PostgreSQL / Redis / SQS)

@Transactional boundaries

Never extract logic out of a @Transactional method into a new @Transactional method on the same class and call it via this.x() expecting a new transaction.

Spring Framework reference docs (Declarative Transaction Management → Using @Transactional): "In proxy mode (which is the default), only external method calls coming in through the proxy are intercepted. This means that self-invocation (in effect, a method within the target object calling another method of the target object) does not lead to an actual transaction at runtime even if the invoked method is marked with @Transactional." The same page notes: "transactional methods in interface-based proxies must always be public" — i.e. a private extracted method cannot carry its own tx.

Fix: extract to a different Spring bean, or switch to AspectJ mode ("Consider using AspectJ mode … if you expect self-invocations to be wrapped with transactions as well").

WebFlux reactive chains

A long Mono/Flux chain is one expression — counting raw lines is misleading. Decompose by extracting each flatMap/map stage that has a distinct business meaning into a private method returning Mono<T> / Flux<T> (e.g. fetchAccount(), validateCreditLimit(), enqueueNotificationEvent()). Do NOT extract one-liner .map(x -> x.toUpper()) stages — that's noise.

Never .block() inside a reactive chain. Never call subscribe() inside a map/flatMap — both lead to leaked subscriptions and uncontrolled execution.

@Service methods

Target ≤ 30 lines, ≤ 3 collaborators per orchestration step. If a service method calls > 5 collaborators, decompose into use-case classes or split the service along the verbs (charge vs. refund, not one giant PaymentService).

Repository methods

Keep them thin; never put business logic in @Query methods or in JdbcTemplate callbacks beyond mapping.

SQS consumers

Separate (1) message parsing/validation, (2) idempotency check (Redis), (3) business action, (4) ack/nack. Each as its own private method; the public @SqsListener reads as a 4-line orchestration.

Examples

❌ Over-decomposed (avoid)

public BigDecimal computeFee(Charge c) {
    return addBaseFee(applyDiscount(taxOf(c)));
}
private BigDecimal taxOf(Charge c) { return c.amount().multiply(TAX_RATE); }
private BigDecimal applyDiscount(BigDecimal v) { return v.multiply(NINE_TENTHS); }
private BigDecimal addBaseFee(BigDecimal v) { return v.add(BASE_FEE); }

Three single-call helpers add zero clarity over the inline expression and force the reader to scroll through four definitions to verify one formula.

✅ Right-sized

public BigDecimal computeFee(Charge c) {
    var tax = c.amount().multiply(TAX_RATE);
    var discounted = tax.multiply(NINE_TENTHS);
    return discounted.add(BASE_FEE);
}

✅ Extraction earns its keep when the name encodes a domain rule

public ChargeResult process(ChargeRequest req) {
    rejectIfBlacklisted(req.payer());                        // distinct policy
    var account = resolveAccount(req.target());              // distinct lookup
    var auth    = authorizeFunds(req.payer(), req.amount()); // distinct domain step
    return submitToLedger(req, account, auth);
}

Research-first trigger (Context7 MCP + web search)

Before writing code for a feature, run a Context7 lookup (and web search if needed) when ANY of these apply:

• Introducing a new library, framework feature, or major version (e.g. Spring Boot 3.5 → 4.x, Spring Security 6.x lambda DSL, Reactor 3.7+ APIs, Java 25 scoped values / structured concurrency / pattern matching for switch).
• Using an API you haven't touched in this repo in the last few commits.
• A new architectural pattern in this codebase (saga, outbox, CDC, circuit breaker, idempotency receiver, token bucket).
• Security-sensitive code: auth, crypto, OTP, PII handling, payment-signature verification, secret management.
• Performance-critical path: hot loops, reactive backpressure tuning, PostgreSQL query plans, Redis Lua scripts, SQS visibility-timeout tuning, JVM tuning.
• Cross-service contract change: new SQS event schema, new HTTP contract, breaking API change.
• AWS service integration beyond plain SQS send/receive (DLQ patterns, FIFO + dedup, IAM scoping).
• The user explicitly asks for "best practice", "idiomatic", "latest", "modern", or "current recommended approach".

Procedure:

1. State briefly: "Triggering research-first: <which trigger>."
2. Call resolve-library-id then get-library-docs (Context7 MCP) for the specific topic, and/or a focused web search for blog posts / conf talks from the last 24 months.
3. Summarise findings in 3–6 bullets, citing the source library/version, before proposing an implementation.
4. Then write code.

Do NOT trigger research-first for:

• Trivial CRUD on an existing entity that already follows a clear repo pattern.
• Renames, formatting, dead-code removal, log-message tweaks.
• Bug fixes where the root cause is visible in a stack trace and the fix is localised.
• Tests that mirror an existing test pattern.
• Anything you can implement by copying the shape of nearby code in the same module with confidence.

If unsure, default to a 60-second Context7 check rather than guessing, but never burn a full research cycle on a one-line fix.

Self-check before submitting

Run mentally on each method you wrote/changed:

• One thing, one abstraction level?
• ≤ 30 lines, ≤ 3 nesting levels, ≤ 5 params? (Java: confirm the borderline ones with the PMD measurement above instead of guessing.)
• Name describes intent without needing the body?
• Would inlining it make the caller HARDER to read? If "no," consider inlining.
• Did I introduce a tx/reactive boundary by accident?

Common mistakes