Java Code Smells

---
name: java-code-smells
description: >
  The detection catalogue for Java code smells: Long Method, God Object, Feature Envy,
  Primitive Obsession, Data Clumps, Shotgun Surgery, Divergent Change, Mysterious Name,
  Mutable and Global Data, Data Class, Loops, Lazy Element, Refused Bequest, boolean
  blindness, null-heavy APIs and leaky abstraction, plus how modern Java changes the list
  and the routing table from a finding to the refactoring that fixes it. Use when auditing
  code for structural problems, before planning a refactoring, when one change keeps fanning
  out across many files, when several refactorings could address one finding, when a switch
  over a sealed type carries a default branch, or when deciding whether a suspect pattern is
  actually a problem. Detection and severity only — refactoring mechanics are
  java-refactoring, navigation-chain depth is java-law-of-demeter, and the economics of
  duplication and premature abstraction are java-dry-kiss-yagni.
---

# Java Code Smells

## Purpose

A smell is evidence, not a verdict. This skill runs a detection pass whose output is a
short, prioritised list of findings — each with the code it names, the evidence, a
severity argument, and an appropriate corrective route. Structural findings name a
java-refactoring technique; demonstrated behavior defects need a separate correction. The failure
modes it prevents: reporting everything pattern-matching a smell (noise the team
ignores), and rewriting code during what was supposed to be a diagnosis.

## Workflow

Use Java 21 without preview as the example baseline. Inspect Maven/Gradle release settings,
toolchains and CI/runtime versions before suggesting records, sealed types or pattern switches;
do not upgrade the project or enable preview features as part of detection. The catalogue
also applies to older Java, with recommendations conditional on its supported language level.

Establish the requested review boundary from the task, then inspect relevant callers, tests,
configuration and documented contracts or prior design decisions. Treat consistent conventions
as evidence of intent, not automatically as requirements; a lone implementation is not policy.
When an unknown changes the recommendation (for example, whether an API has external consumers),
look for repository evidence first, then ask that focused question if still unresolved. Continue
independent findings and state the assumption and what would change the recommendation.

1. **Scan for signals, not smells.** Size outliers (methods, classes, parameter lists),
   change history (`git log --follow` on files that appear in every PR), duplication,
   and dependency fan-in/fan-out. Signals say where to look; the catalogue says what
   you found. Inspect representative diffs before claiming recurring semantic co-change;
   separate formatting, mechanical migrations and generated-file churn from changes to the
   underlying rule. Their file count alone does not prove Shotgun Surgery or Divergent Change.
2. **Classify against the catalogue** — read `references/catalogue-within.md` for
   findings inside one class, `references/catalogue-between.md` for findings about
   coupling between classes. Check the smell's false positives before recording it.
3. **Weigh severity from risk, not size.** Combine change frequency and blast radius with
   business/security criticality, defect evidence, test/observability confidence, ownership,
   reversibility and migration cost. A rarely edited authorisation or money-movement path may
   outrank a frequently edited formatter. Repository history is evidence, not a veto.
4. **Record findings, do not fix them.** Each finding: location, evidence, consequence,
   severity argument and corrective route. For a structural finding, name the smell and use
   `references/smell-to-refactoring.md` to select a technique or explain why no refactoring is
   warranted. For a demonstrated behavior defect, follow the correction rule below. Pass the
   evidence, contracts and affected tests into any subsequent fix pass; expect checks of the
   identified risk, not merely cleaner code. If the user already authorized fixes, continue
   under the receiving skill's safety workflow without another approval gate. If that skill is
   unavailable, retain the concrete recommendation and verification needs; the detection result
   must remain usable on its own.
5. **Close the bounded pass.** Recheck each reported finding against counter-evidence and say
   what would falsify it or verify the recommended correction. Deliver the prioritised findings,
   reviewed scope and material evidence gaps; no findings is valid when candidates were falsified.
   Stop when the requested scope is covered and supported findings have an actionable route;
   an unresolved lead should name its next discriminating check, not widen the audit indefinitely.

If history is shallow or unavailable, say which change-pressure claims cannot be established.
Use source-level evidence for current coupling, but do not invent recurring co-change or defect
history. Name what additional evidence would distinguish a finding from a monitor-only lead.

## Rules

- No finding without evidence a reviewer can check: a metric, a diff that fanned out, a
  duplicated block's two locations, or a contract violation with its triggering path.
  "This looks wrong" is not a finding.
- If inspection demonstrates a correctness defect, report expected versus actual behavior and
  the triggering input/path separately from its structural explanation. Refactoring preserves
  behavior; extraction or renaming alone cannot be credited with correcting it. Route uncertain
  failure attribution to `debugging` with the reproduction, contract and competing explanations;
  expect a supported cause and correction/verification plan. If unavailable, retain the bounded
  finding and next discriminating check. Execute fixes only within the already authorized scope.
- Stable code gets a lower change-pressure score, not immunity. Dormant compatibility,
  security, concurrency and data-integrity defects still warrant findings when their impact and
  evidence are strong; absence of tickets is weak evidence when failures are silent.
- One structural cause often shows as several smells (a God Object produces Feature
  Envy in its neighbours and Shotgun Surgery in its callers). Report the cause once,
  not each symptom separately.
- Judge unchecked dispatch and the required variant-review policy, not the `switch`
  keyword or absence of `default`. A cohesive exhaustive switch may be adequate; a
  catch-all type pattern can also hide a new variant. Read `references/modern-java.md`
  before flagging any switch, record, or Optional usage.
- A comment apologising for code ("hack", "careful here") is a search lead, not a finding.
  Preserve comments that encode an invariant, upstream defect, compatibility constraint or
  measured workaround; report the underlying structure only when independently evidenced.
- Never bundle a fix into the detection pass. Detection changes no code.

## Severity and decision record

For each candidate, record this compact tuple:

```text
location + structural signal
evidence and counter-evidence
change pressure + impact if wrong
test/observability confidence
plausible refactoring and migration surface
decision: finding | monitor | no action
```

Do not multiply ordinal scores and pretend the result is quantitative risk. Use the dimensions
to expose the argument, then rank findings relative to this repository:

| Priority      | Typical evidence                                                                                                                   |
| ------------- | ---------------------------------------------------------------------------------------------------------------------------------- |
| Critical/high | Security, money or data-integrity failure; repeated incidents; unsafe concurrent state; change is already blocked by the structure |
| Medium        | Recurring co-change/defects with a bounded refactoring seam and adequate characterization tests                                    |
| Low/monitor   | Readability cost with little change pressure, speculative future benefit, or migration cost larger than demonstrated harm          |
| No action     | Intentional boundary/representation, generated code, framework contract, or candidate falsified by ownership/change evidence       |

Before recommending a move, identify externally observed contracts: serialized fields, database
mapping, reflection, dependency injection, native-image configuration and module exports. A
smell inside such a boundary may be real while the safe recommendation is staged migration,
not immediate cleanup.

## References

- [Catalogue: within a class](references/catalogue-within.md) — Long Method, Large
  Class, Primitive Obsession, Data Clumps, Temporary Field, Duplicate Code, Dead Code,
  comments-as-deodorant, boolean blindness, Speculative Generality, Mysterious Name,
  Long Parameter List, Mutable Data, Loops, Lazy Element.
- [Catalogue: between classes](references/catalogue-between.md) — Feature Envy,
  Shotgun Surgery, Divergent Change, Message Chains, Middle Man, Refused Bequest,
  Inappropriate Intimacy, Switch Statements, null-heavy APIs, leaky abstraction, Global
  Data, Alternative Classes with Different Interfaces, Data Class.
- [Smell → refactoring](references/smell-to-refactoring.md) — the routing table from a
  recorded finding to the java-refactoring techniques that address it, what decides
  between competing techniques, the sequences that must run in order, and when the
  correct output is no refactoring at all. Read at step 4, when turning findings into
  recommendations.
- [Modern Java: dissolved and created smells](references/modern-java.md) — what
  records, sealed types and Optional removed from the classic catalogue and what they
  added. Read before flagging switches, records, or Optional chains.
- [A worked smell pass](references/worked-pass.md) — one realistic service audited end
  to end: signals, findings, severity weighing, and the false positive that was
  deliberately not reported. Read when unsure how to weigh or phrase findings.
- [Primitive confusion check](scripts/primitive-obsession/verify.sh) — run with a POSIX shell
  and JDK 21+ when assessing whether distinct identifier types prevent a real argument swap.
  It compiles with `--release 21`, runs the before/after cases and checks the intentional
  compile failure. This demonstrates type safety, not application behavior or performance.