clean-code
Skill0 starsChanged 5 months agoMITView on GitHub
Clean code review lens (Martin-style). Use when reviewing code for readability, naming, function design, error handling, and code smells. TRIGGER when: reviewing code quality, doing code review, repo health check, or user asks about clean code principles. DO NOT TRIGGER when: user is focused on domain modelling (use /core-skills:ddd) or test-driving new code (use /core-skills:tdd).
108 other projects use a version of this.
Adopted by
Most recently taken up by
- sbluemin/fleet-harnesstheir own version, first seen Nov 8, 2026
- nohemi20231/design-skillstheir own version, first seen Nov 8, 2026
- MammadovEshgin/clean-code-skilltheir own version, first seen Nov 8, 2026
- Mahesh-Bishnoi/skillstheir own version, first seen Nov 8, 2026
- Bruno-Cunha-Souza/ValarMindSkillstheir own version, first seen Nov 8, 2026
- kana-consultant/kana-best-practice-engineeringtheir own version, first seen Nov 8, 2026
Used with
Projects with it also use intelligent-routing, nextjs-react-expert and 10 more.
Other files people use alongside this one.
Files found in the same public projects as a version of this one, more often than they turn up anywhere else. Each project counts once, large collections count for less, and this file's own repository is left out.
SKB3002Skillintelligent-routing
Automatic agent selection and intelligent task routing. Analyzes user requests and automatically selects the best specialist agent(s) without requiring explicit user mentions.
Also in 5 projects that use this
- SKB3002Skill
nextjs-react-expert
React and Next.js performance optimization from Vercel Engineering. Use when building React components, optimizing performance, eliminating waterfalls, reducing bundle size, reviewing code for performance issues, or implementing server/client-side optimizations.
Also in 3 projects that use this
xfstudioSkillbash-pro
Master of defensive Bash scripting for production automation, CI/CD
Also in 4 projects that use this
- xfstudioSkill
code-refactoring-tech-debt
You are a technical debt expert specializing in identifying, quantifying, and prioritizing technical debt in software projects. Analyze the codebase to uncover debt, assess its impact, and create acti
Also in 4 projects that use this
flying-diceSkillclean-code-review
Multi-agent clean code audit — each principle gets its own agent
Also in 3 projects that use this
goca-seSkillbatch-operations
Apply a change safely across many files — refactors touching more than 5 files, library or API migrations, project-wide renames of symbols/imports/paths, or adding boilerplate to many files. Use when "do it all at once" risks context overflow or silent failures.
Also in 3 projects that use this
- +4 more from xfstudio
Other versions108 projects use a version of this.
Word for word · 0
None found.
Edited · 0
None found.
Wrote their own version · 108
- superplanehq/superplane★ 9.1ksame name
- vudovn/ag-kit★ 8.2ksame name
- vudovn/antigravity-kit★ 8.2ksame name
- Openpanel-dev/openpanel★ 7.3ksame name
- ciembor/agent-rules-books★ 2.8ksame name
- wondelai/skills★ 2.6ksame name
- decebals/claude-code-java★ 763same name
- operately/operately★ 618same name
- liza-mas/liza★ 418same name
- Mindrally/skills★ 275same name
98 more not listed
How matches are found. Word for word: the exact same text. Edited: shares passages with this file (“99% same” means 99% of its five-word phrases also appear here; 10% or more counts). Their own version: similar meaning, or, weakest, the same name. We can’t tell which project had it first. Projects are counted among the 200 most-starred candidates, and mentions are links to the repository we found, up to 50 per site, so both counts are a floor.
In this repository13 other files it gives its agents.
- CLAUDE.mdCLAUDE.md
- create-design-briefSkill
- dddSkill
- goosSkill
- ingest-designSkill
- kaizenSkill
- panelSkill
- researchSkill
- review-loopSkill
- software-designSkill
- sopSkill
- synthesizeSkill
- tddSkill
The file
We split this skill into the rules it gives an agent
5 rules. 0 are common, 5 are distinctive, and 0 are very specific to this project.
0
Common
Most of the 101 edited versions we read keep these.
5
Distinctive
This author's own choices. Worth a look.
0
Very specific
Tied to their project's names, files or tools. Probably skip.
- Common
- Distinctive
- Very specific
- Distinctive: line 12
When reviewing names, apply these rules in order:
- Distinctive: line 109
When reviewing code that uses third-party libraries or APIs:
- Distinctive: line 112
Learning tests: If the codebase uses a third-party API, check for tests that document expected behavior of that API.
- Distinctive: line 113
Adapter at the boundary: When your code needs an interface that doesn't exist yet (e.g., from an external team), define the interface you …
- Distinctive: line 117
After scanning for individual smells, check these cross-cutting concerns:
“Common” is measured: kept by at least 6 in 10 of the 101 edited versions of this file we could read. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.
Files
Clean Code — Review Lens
You are reviewing code through the Clean Code lens. Scan for the smells and violations below. When you find one, name the specific smell (e.g., "G14: Feature Envy") and state the fix.
Naming
When reviewing names, apply these rules in order:
| When you see... | Flag as... | Fix |
| Name requires reading the implementation to understand | N1: Non-descriptive name | Rename to reveal intent |
Two names differ only in spelling (XYZController / XYZManager) | Meaningless distinction | Pick one term per concept, use it consistently |
Name communicates implementation (phoneNumber on an abstract interface) | N2: Wrong abstraction level | Rename to the concept (connectionLocator) |
| Name doesn't use domain/pattern vocabulary when it could | N3: Non-standard nomenclature | Use pattern name (Decorator, Factory) or domain term |
| Name is ambiguous about what the function actually does | N4: Ambiguous name | Rename to include the full scope of behavior |
| Short name in a large scope, or long name in a tiny scope | N5: Scope-length mismatch | i fine in 3-line loop; module-level names should be descriptive |
Hungarian notation, m_ prefix, I prefix on interfaces | N6: Encoding in name | Remove the encoding; modern tooling makes it redundant |
Name hides a side effect (getX() that also creates X) | N7: Hidden side effect | Rename to describe all effects (createOrReturnX) |
Same word used for two different operations (e.g., add for both append and insert) | Pun | Use distinct words for distinct operations |
Functions
| When you see... | Flag as... | Fix |
| Function body has sections at different abstraction levels | G34: Mixed abstraction | Extract lower-level operations into named helper functions |
| Function does more than one thing (you can extract a meaningfully-named function from it) | G30: Does too much | Extract until each function does one thing |
| Function takes a boolean argument that selects behavior | F3/G15: Flag/selector arg | Split into two functions, one per branch |
| Function has 3+ parameters | F1: Too many arguments | Wrap related args into an object, or restructure |
| Function modifies an argument instead of returning a value | F2: Output argument | Have the owning object perform the mutation via method call |
| Function both changes state AND returns a value | Command-query violation | Split: one function to change state, another to query it |
| Function body is a try/catch with logic inside both blocks | Unseparated error handling | Extract try body and catch body into named functions |
| Function is never called | F4: Dead function | Delete it; VCS remembers |
if/else or switch dispatches on type in multiple places | G23: Missing polymorphism | Replace with polymorphic objects; at most one switch to create them |
Comments
| When you see... | Flag as... | Fix |
| Comment restates what the code already says | C3: Redundant comment | Delete it |
| Comment contradicts the code | C2: Obsolete comment | Update or delete it |
| Comment contains change history, author, date | C1: Inappropriate info | Remove; this belongs in version control |
| Commented-out code | C5: Commented-out code | Delete it; VCS remembers |
| Comment is vague, rambling, or ungrammatical | C4: Poorly written | Rewrite clearly and concisely, or delete if unnecessary |
| Doc-comment boilerplate that adds nothing beyond the signature | Noise | Delete it; only write comments that say what code cannot |
Good comments to preserve: legal notices, explanation of intent behind a non-obvious decision, clarification of an obscure API call, warning of consequences, TODO with ticket reference, amplification of something easily overlooked.
Error Handling
| When you see... | Flag as... | Fix |
Error signalled by return code or special value (e.g., -1, null) | Return-code error style | Use exceptions to separate happy path from error path |
| Null returned from a function | Null return | Return empty collection, Optional, or a Special Case object |
| Null passed as a function argument | Null argument | Forbid by convention; fail fast with clear error if received |
| Catch block is empty or logs and continues | Swallowed exception | Handle it, re-raise with context, or propagate |
| Exception message lacks context about what failed | Context-free exception | Include the operation attempted and the failure reason |
| Third-party API exceptions leak through the codebase | Unwrapped boundary | Wrap third-party calls; translate their exceptions into your domain exceptions |
| Code checks for a special condition and returns null/throws | Missing Special Case | Create a Special Case object that encapsulates the default behavior |
Structure and Coupling
| When you see... | Flag as... | Fix |
Method chains: a.getB().getC().doSomething() | G36: Transitive navigation (Law of Demeter) | a.doSomething() — ask the immediate collaborator |
| Method mostly uses another object's getters to do work | G14: Feature Envy | Move the method to the class whose data it uses |
| Class has multiple unrelated reasons to change | SRP violation | Split into focused classes, one responsibility each |
| Class has low cohesion (methods use different subsets of instance variables) | Low cohesion | Split into multiple classes, each highly cohesive |
| Constants/functions placed in a convenient but wrong module | G13: Artificial coupling | Move to the module where they logically belong |
| Base class references or imports its own derived classes | G7: Base depends on derivatives | Invert the dependency; base should not know about derivatives |
| Implementation details in an abstract interface | G6: Wrong abstraction level | Push implementation details down to derived classes/modules |
| Two functions must be called in a specific order, but nothing enforces it | G31: Hidden temporal coupling | Use bucket brigade: each function returns what the next needs |
| Data or constants that could change are buried in low-level code | G35: Configuration at wrong level | Move configurable values to high-level initialization |
| A dependency exists logically but is not explicit in the code | G22: Logical dependency not physical | Make the dependency explicit via parameter or interface |
| Class/module exposes too many methods, variables, or constants | G8: Too much information | Minimize the public interface; hide data, utilities, constants |
| Static method that could plausibly need polymorphic dispatch | G18: Inappropriate static | Make it an instance method; only use static when polymorphism is impossible |
Duplication and Dead Code
| When you see... | Flag as... | Fix |
| Identical or near-identical code in multiple places | G5: Duplication | Extract into a shared function or class |
Repeated if/else or switch chains testing the same conditions | G5: Structural duplication | Replace with polymorphism or Template Method |
| Modules with similar algorithms but different details | G5: Algorithmic duplication | Extract common structure using Template Method or Strategy |
Code in an if that can never be true, catch that can never fire | G9: Dead code | Delete it |
| Unused variables, unused imports, empty default constructors | G12: Clutter | Delete them |
Formatting and Clarity
| When you see... | Flag as... | Fix |
| Public function at bottom, private helpers at top | Inverted newspaper order | Put high-level public functions first, helpers below them |
| Variable declared far from where it is used | G10: Vertical separation | Move declaration close to first usage |
| No blank lines between distinct concepts in a function | Missing vertical openness | Add blank lines to separate logical sections |
| Related functions scattered across the file | Conceptual disaffinity | Group related functions together; caller above callee |
| Magic number or magic string literal | G25: Magic number | Extract to a named constant |
Complex boolean expression inline in if | G28: Unencapsulated conditional | Extract to a well-named predicate function |
Negative conditional (if (!isNotFound())) | G29: Negative conditional | Rewrite as positive (if (isFound())) |
| Dense expression with unclear intent | G16: Obscured intent | Break into explanatory variables (G19) with meaningful names |
Boundary arithmetic (level + 1) repeated in multiple places | G33: Unencapsulated boundary | Extract to a named variable (nextLevel = level + 1) |
| Similar operations use inconsistent patterns or naming | G11: Inconsistency | Adopt one convention and apply it uniformly |
Boundaries
When reviewing code that uses third-party libraries or APIs:
- Wrap, don't leak: Third-party types should not spread through the codebase. Wrap the third-party API in your own class that exposes only what you need.
- Learning tests: If the codebase uses a third-party API, check for tests that document expected behavior of that API. Recommend adding them if missing.
- Adapter at the boundary: When your code needs an interface that doesn't exist yet (e.g., from an external team), define the interface you want, then write an adapter when the real API arrives.
Review Checklist
After scanning for individual smells, check these cross-cutting concerns:
- Obvious behavior implemented? (G2) — Does every function/class do what its name promises?
- Boundary conditions tested? (G3) — Are edge cases handled, not just the happy path?
- No overridden safeties? (G4) — Are warnings enabled, tests running, type checks in place?
- Precise? (G26) — Are types correct (no float for currency), concurrency handled, null cases addressed?
- Algorithm understood? (G21) — Is the code obviously correct, not just "passes the tests I thought of"?
- Structure enforces design? (G27) — Are constraints enforced by the type system or architecture, not just naming conventions?
- Don't be arbitrary (G32) — Does every structural choice have a reason? Would a new contributor understand why?
Does it work?
Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.
No reviews yet
People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.
- It worked
- Partly
- It didn't
Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.
Your trail
Files you open will collect here, so you can find them again.
Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.
Now do this for your own.
Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.