solid

SOLID Review

A design finding is worth reporting only when the design already costs something or will on the next ordinary change — a bug, an inconsistency, a change that has to touch many places, a unit that cannot be tested. Style, naming taste, "could be more elegant" and textbook purity are never findings.

Scope

Review the changed code and what it calls, read whole — not only the diff hunks. Judge the change against the conventions the repository already follows (its AGENTS.md/CLAUDE.md, existing seams and patterns). A repo that keeps small scripts procedural is not violating SOLID by doing so.

Rules

SRP — Single Responsibility

One unit (function, class, module, service) has to change for two unrelated reasons. Name both reasons in the finding.

• Finding: an HTTP controller that validates input, builds SQL, renders e-mail and spawns a child process; a Drupal form submit handler that also syncs to a remote API.
• Not a finding: a long function that does one thing in several steps.

OCP — Open/Closed

Adding the next variant (a check, a backend, a provider, a content type) requires editing a switch/if chain in several places instead of one registration point (a map, a plugin type, a tagged service, an event subscriber).

• Cite every place the next variant would have to touch.
• Not a finding: a single match in one place that is the registration point.

LSP — Liskov Substitution

An implementation that throws, no-ops or silently changes meaning for part of the contract it claims, so that callers must know which concrete class they have (instanceof checks, "not supported" exceptions, a subclass that ignores an argument).

ISP — Interface Segregation

Callers are forced to depend on methods they never use, or implementers must stub out half an interface. Cite the unused methods and the stubbing implementers.

DIP — Dependency Inversion

Business logic hard-wired to a concrete I/O dependency (network, filesystem, database, clock, child process, a static \Drupal::service() call inside a class that supports injection) so that it cannot be tested or swapped without it — where the codebase already has a seam for that dependency elsewhere (DI container, an injected client, a clock service). Without an existing seam, report it only if it already blocks a test the change needs.