--- name: clean-code model: standard category: testing description: Pragmatic coding standards for writing clean, maintainable code — naming, functions, structure, anti-patterns, and pre-edit safety checks. Use when writing new code, refactoring existing code, reviewing code quality, or establishing coding standards. version: 2.0 --- # Clean Code > Be **concise, direct, and solution-focused**. Clean code reads like well-written prose — every name reveals intent, every function does one thing, and every abstraction earns its place. ## Installation ### OpenClaw / Moltbot / Clawbot ```bash npx clawhub@latest install clean-code ``` --- ## Core Principles | Principle | Rule | Practical Test | |-----------|------|----------------| | **SRP** | Single Responsibility — each function/class does ONE thing | "Can I describe what this does without using 'and'?" | | **DRY** | Don't Repeat Yourself — extract duplicates, reuse | "Have I written this logic before?" | | **KISS** | Keep It Simple — simplest solution that works | "Is there a simpler way to achieve this?" | | **YAGNI** | You Aren't Gonna Need It — don't build unused features | "Does anyone need this right now?" | | **Boy Scout** | Leave code cleaner than you found it | "Is this file better after my change?" | --- ## Naming Rules Names are the most important documentation. A good name eliminates the need for a comment. | Element | Convention | Bad | Good | |---------|------------|-----|------| | **Variables** | Reveal intent | `n`, `d`, `tmp` | `userCount`, `elapsed`, `activeUsers` | | **Functions** | Verb + noun | `user()`, `calc()` | `getUserById()`, `calculateTotal()` | | **Booleans** | Question form | `active`, `flag` | `isActive`, `hasPermission`, `canEdit` | | **Constants** | SCREAMING_SNAKE | `max`, `timeout` | `MAX_RETRY_COUNT`, `REQUEST_TIMEOUT_MS` | | **Classes** | Noun, singular | `Manager`, `Data` | `UserRepository`, `OrderService` | | **Enums** | PascalCase values | `'pending'` string | `Status.Pending` | > **Rule:** If you need a comment to explain a name, rename it. ### Naming Anti-Patterns | Anti-Pattern | Problem | Fix | |--------------|---------|-----| | Cryptic abbreviations (`usrMgr`, `cfg`) | Unreadable in 6 months | Spell it out — IDE autocomplete makes long names free | | Generic names (`data`, `info`, `item`, `handler`) | Says nothing about purpose | Use domain-specific names that reveal intent | | Misleading names (`getUserList` returns one user) | Actively deceives readers | Match name to behavior, or change the behavior | | Hungarian notation (`strName`, `nCount`, `IUser`) | Redundant with type system | Let TypeScript/IDE show types; names describe purpose | --- ## Function Rules | Rule | Guideline | Why | |------|-----------|-----| | **Small** | Max 20 lines, ideally 5-10 | Fits in your head | | **One Thing** | Does one thing, does it well | Testable and nameable | | **One Level** | One level of abstraction per function | Readable top to bottom | | **Few Args** | Max 3 arguments, prefer 0-2 | Easy to call correctly | | **No Side Effects** | Don't mutate inputs unexpectedly | Predictable behavior | ### Guard Clauses Flatten nested conditionals with early returns. Never nest deeper than 2 levels. ```typescript // BAD — 5 levels deep function processOrder(order: Order) { if (order) { if (order.items.length > 0) { if (order.customer) { if (order.customer.isVerified) { return submitOrder(order); } } } } throw new Error('Invalid order'); } // GOOD — guard clauses flatten the structure function processOrder(order: Order) { if (!order) throw new Error('No order'); if (!order.items.length) throw new Error('No items'); if (!order.customer) throw new Error('No customer'); if (!order.customer.isVerified) throw new Error('Customer not verified'); return submitOrder(order); } ``` ### Parameter Objects When a function needs more than 3 arguments, use an options object. ```typescript // BAD — too many parameters, order matters createUser('John', 'Doe', 'john@example.com', 'secret', 'admin', 'Engineering'); // GOOD — self-documenting options object createUser({ firstName: 'John', lastName: 'Doe', email: 'john@example.com', password: 'secret', role: 'admin', department: 'Engineering', }); ``` --- ## Code Structure Patterns | Pattern | When to Apply | Benefit | |---------|--------------|---------| | **Guard Clauses** | Edge cases at function start | Flat, readable flow | | **Flat > Nested** | Any nesting beyond 2 levels | Reduced cognitive load | | **Composition** | Complex operations | Small, testable pieces | | **Colocation** | Related code across files | Easier to find and change | | **Extract Function** | Comments separating "sections" | Self-documenting code | ### Composition Over God Functions ```typescript // BAD — god function doing everything async function processOrder(order: Order) { // Validate... (15 lines) // Calculate totals... (15 lines) // Process payment... (10 lines) // Send notifications... (10 lines) // Update inventory... (10 lines) return { success: true }; } // GOOD — composed of small, focused functions async function processOrder(order: Order) { validateOrder(order); const totals = calculateOrderTotals(order); const payment = await processPayment(order.customer, totals); await sendOrderConfirmation(order, payment); await updateInventory(order.items); return { success: true, orderId: payment.orderId }; } ``` --- ## Return Type Consistency Functions should return consistent types. Use discriminated unions for multiple outcomes. ```typescript // BAD — returns different types function getUser(id: string) { const user = database.find(id); if (!user) return false; // boolean if (user.isDeleted) return null; // null return user; // User } // GOOD — discriminated union type GetUserResult = | { status: 'found'; user: User } | { status: 'not_found' } | { status: 'deleted' }; function getUser(id: string): GetUserResult { const user = database.find(id); if (!user) return { status: 'not_found' }; if (user.isDeleted) return { status: 'deleted' }; return { status: 'found', user }; } ``` --- ## Anti-Patterns | Anti-Pattern | Problem | Fix | |--------------|---------|-----| | Comment every line | Noise obscures signal | Delete obvious comments; comment *why*, not *what* | | Helper for one-liner | Unnecessary indirection | Inline the code | | Factory for 2 objects | Over-engineering | Direct instantiation | | `utils.ts` with 1 function | Junk drawer file | Put code where it's used | | Deep nesting | Unreadable flow | Guard clauses and early returns | | Magic numbers | Unclear intent | Named constants | | God functions | Untestable, unreadable | Split by responsibility | | Commented-out code | Dead code confusion | Delete it; git remembers | | TODO sprawl | Never gets done | Track in issue tracker, not code | | Premature abstraction | Wrong abstraction is worse than none | Wait for 3+ duplicates before abstracting | | Copy-paste programming | Duplicated bugs | Extract shared logic | | Exception-driven control flow | Slow and confusing | Use explicit conditionals | | Stringly-typed code | Typos and missed cases | Use enums or union types | | Callback hell | Pyramid of doom | Use async/await | --- ## Pre-Edit Safety Check Before changing any file, answer these questions to avoid cascading breakage: | Question | Why | |----------|-----| | **What imports this file?** | Dependents might break on interface changes | | **What does this file import?** | You might need to update the contract | | **What tests cover this?** | Tests might fail — update them alongside code | | **Is this a shared component?** | Multiple consumers means wider blast radius | ``` File to edit: UserService.ts ├── Who imports this? → UserController.ts, AuthController.ts ├── Do they need changes too? → Check function signatures └── What tests cover this? → UserService.test.ts ``` > **Rule:** Edit the file + all dependent files in the SAME task. Never leave broken imports or missing updates. --- ## Self-Check Before Completing Before marking any task complete, verify: | Check | Question | |-------|----------| | **Goal met?** | Did I do exactly what was asked? | | **Files edited?** | Did I modify all necessary files, including dependents? | | **Code works?** | Did I verify the change compiles and runs? | | **No errors?** | Do lint and type checks pass? | | **Nothing forgotten?** | Any edge cases or dependent files missed? | --- ## NEVER Do 1. **NEVER add comments that restate the code** — if the code needs a comment to explain *what* it does, rename things until it doesn't 2. **NEVER create abstractions for fewer than 3 use cases** — premature abstraction is worse than duplication 3. **NEVER leave commented-out code in the codebase** — delete it; version control exists for history 4. **NEVER write functions longer than 20 lines** — extract sub-functions until each does one thing 5. **NEVER nest deeper than 2 levels** — use guard clauses, early returns, or extract functions 6. **NEVER use magic numbers or strings** — define named constants with clear semantics 7. **NEVER edit a file without checking what depends on it** — broken imports and missing updates are the most common source of bugs in multi-file changes 8. **NEVER leave a task with failing lint or type checks** — fix all errors before marking complete --- ## References Detailed guides for specific clean code topics: | Reference | Description | |-----------|-------------| | [Anti-Patterns](references/anti-patterns.md) | 21 common mistakes with bad/good code examples across naming, functions, structure, and comments | | [Code Smells](references/code-smells.md) | Classic code smells catalog with detection patterns — Bloaters, OO Abusers, Change Preventers, Dispensables, Couplers | | [Refactoring Catalog](references/refactoring-catalog.md) | Essential refactoring patterns with before/after examples and step-by-step mechanics |