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
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.
// 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.
// 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
// 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.
// 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
- NEVER add comments that restate the code — if the code needs a comment to explain what it does, rename things until it doesn't
- NEVER create abstractions for fewer than 3 use cases — premature abstraction is worse than duplication
- NEVER leave commented-out code in the codebase — delete it; version control exists for history
- NEVER write functions longer than 20 lines — extract sub-functions until each does one thing
- NEVER nest deeper than 2 levels — use guard clauses, early returns, or extract functions
- NEVER use magic numbers or strings — define named constants with clear semantics
- 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
- 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 |
21 common mistakes with bad/good code examples across naming, functions, structure, and comments |
| Code Smells |
Classic code smells catalog with detection patterns — Bloaters, OO Abusers, Change Preventers, Dispensables, Couplers |
| Refactoring Catalog |
Essential refactoring patterns with before/after examples and step-by-step mechanics |
1---2name: clean-code-review3description: Clean code review and maintainability guidance for naming, small functions, responsibility boundaries, anti-patterns, code smells, KISS/DRY/YAGNI, and refactoring recommendations. Use when writing, reviewing, or refactoring code for clarity, readability, maintainability, and clean-code principles.4---56# Clean Code78> 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.91011## Installation1213### OpenClaw / Moltbot / Clawbot1415```bash16npx clawhub@latest install clean-code17```181920---2122## Core Principles2324| Principle | Rule | Practical Test |25|-----------|------|----------------|26| **SRP** | Single Responsibility — each function/class does ONE thing | "Can I describe what this does without using 'and'?" |27| **DRY** | Don't Repeat Yourself — extract duplicates, reuse | "Have I written this logic before?" |28| **KISS** | Keep It Simple — simplest solution that works | "Is there a simpler way to achieve this?" |29| **YAGNI** | You Aren't Gonna Need It — don't build unused features | "Does anyone need this right now?" |30| **Boy Scout** | Leave code cleaner than you found it | "Is this file better after my change?" |3132---3334## Naming Rules3536Names are the most important documentation. A good name eliminates the need for a comment.3738| Element | Convention | Bad | Good |39|---------|------------|-----|------|40| **Variables** | Reveal intent | `n`, `d`, `tmp` | `userCount`, `elapsed`, `activeUsers` |41| **Functions** | Verb + noun | `user()`, `calc()` | `getUserById()`, `calculateTotal()` |42| **Booleans** | Question form | `active`, `flag` | `isActive`, `hasPermission`, `canEdit` |43| **Constants** | SCREAMING_SNAKE | `max`, `timeout` | `MAX_RETRY_COUNT`, `REQUEST_TIMEOUT_MS` |44| **Classes** | Noun, singular | `Manager`, `Data` | `UserRepository`, `OrderService` |45| **Enums** | PascalCase values | `'pending'` string | `Status.Pending` |4647> **Rule:** If you need a comment to explain a name, rename it.4849### Naming Anti-Patterns5051| Anti-Pattern | Problem | Fix |52|--------------|---------|-----|53| Cryptic abbreviations (`usrMgr`, `cfg`) | Unreadable in 6 months | Spell it out — IDE autocomplete makes long names free |54| Generic names (`data`, `info`, `item`, `handler`) | Says nothing about purpose | Use domain-specific names that reveal intent |55| Misleading names (`getUserList` returns one user) | Actively deceives readers | Match name to behavior, or change the behavior |56| Hungarian notation (`strName`, `nCount`, `IUser`) | Redundant with type system | Let TypeScript/IDE show types; names describe purpose |5758---5960## Function Rules6162| Rule | Guideline | Why |63|------|-----------|-----|64| **Small** | Max 20 lines, ideally 5-10 | Fits in your head |65| **One Thing** | Does one thing, does it well | Testable and nameable |66| **One Level** | One level of abstraction per function | Readable top to bottom |67| **Few Args** | Max 3 arguments, prefer 0-2 | Easy to call correctly |68| **No Side Effects** | Don't mutate inputs unexpectedly | Predictable behavior |6970### Guard Clauses7172Flatten nested conditionals with early returns. Never nest deeper than 2 levels.7374```typescript75// BAD — 5 levels deep76function processOrder(order: Order) {77 if (order) {78 if (order.items.length > 0) {79 if (order.customer) {80 if (order.customer.isVerified) {81 return submitOrder(order);82 }83 }84 }85 }86 throw new Error('Invalid order');87}8889// GOOD — guard clauses flatten the structure90function processOrder(order: Order) {91 if (!order) throw new Error('No order');92 if (!order.items.length) throw new Error('No items');93 if (!order.customer) throw new Error('No customer');94 if (!order.customer.isVerified) throw new Error('Customer not verified');9596 return submitOrder(order);97}98```99100### Parameter Objects101102When a function needs more than 3 arguments, use an options object.103104```typescript105// BAD — too many parameters, order matters106createUser('John', 'Doe', 'john@example.com', 'secret', 'admin', 'Engineering');107108// GOOD — self-documenting options object109createUser({110 firstName: 'John',111 lastName: 'Doe',112 email: 'john@example.com',113 password: 'secret',114 role: 'admin',115 department: 'Engineering',116});117```118119---120121## Code Structure Patterns122123| Pattern | When to Apply | Benefit |124|---------|--------------|---------|125| **Guard Clauses** | Edge cases at function start | Flat, readable flow |126| **Flat > Nested** | Any nesting beyond 2 levels | Reduced cognitive load |127| **Composition** | Complex operations | Small, testable pieces |128| **Colocation** | Related code across files | Easier to find and change |129| **Extract Function** | Comments separating "sections" | Self-documenting code |130131### Composition Over God Functions132133```typescript134// BAD — god function doing everything135async function processOrder(order: Order) {136 // Validate... (15 lines)137 // Calculate totals... (15 lines)138 // Process payment... (10 lines)139 // Send notifications... (10 lines)140 // Update inventory... (10 lines)141 return { success: true };142}143144// GOOD — composed of small, focused functions145async function processOrder(order: Order) {146 validateOrder(order);147 const totals = calculateOrderTotals(order);148 const payment = await processPayment(order.customer, totals);149 await sendOrderConfirmation(order, payment);150 await updateInventory(order.items);151 return { success: true, orderId: payment.orderId };152}153```154155---156157## Return Type Consistency158159Functions should return consistent types. Use discriminated unions for multiple outcomes.160161```typescript162// BAD — returns different types163function getUser(id: string) {164 const user = database.find(id);165 if (!user) return false; // boolean166 if (user.isDeleted) return null; // null167 return user; // User168}169170// GOOD — discriminated union171type GetUserResult =172 | { status: 'found'; user: User }173 | { status: 'not_found' }174 | { status: 'deleted' };175176function getUser(id: string): GetUserResult {177 const user = database.find(id);178 if (!user) return { status: 'not_found' };179 if (user.isDeleted) return { status: 'deleted' };180 return { status: 'found', user };181}182```183184---185186## Anti-Patterns187188| Anti-Pattern | Problem | Fix |189|--------------|---------|-----|190| Comment every line | Noise obscures signal | Delete obvious comments; comment *why*, not *what* |191| Helper for one-liner | Unnecessary indirection | Inline the code |192| Factory for 2 objects | Over-engineering | Direct instantiation |193| `utils.ts` with 1 function | Junk drawer file | Put code where it's used |194| Deep nesting | Unreadable flow | Guard clauses and early returns |195| Magic numbers | Unclear intent | Named constants |196| God functions | Untestable, unreadable | Split by responsibility |197| Commented-out code | Dead code confusion | Delete it; git remembers |198| TODO sprawl | Never gets done | Track in issue tracker, not code |199| Premature abstraction | Wrong abstraction is worse than none | Wait for 3+ duplicates before abstracting |200| Copy-paste programming | Duplicated bugs | Extract shared logic |201| Exception-driven control flow | Slow and confusing | Use explicit conditionals |202| Stringly-typed code | Typos and missed cases | Use enums or union types |203| Callback hell | Pyramid of doom | Use async/await |204205---206207## Pre-Edit Safety Check208209Before changing any file, answer these questions to avoid cascading breakage:210211| Question | Why |212|----------|-----|213| **What imports this file?** | Dependents might break on interface changes |214| **What does this file import?** | You might need to update the contract |215| **What tests cover this?** | Tests might fail — update them alongside code |216| **Is this a shared component?** | Multiple consumers means wider blast radius |217218```219File to edit: UserService.ts220├── Who imports this? → UserController.ts, AuthController.ts221├── Do they need changes too? → Check function signatures222└── What tests cover this? → UserService.test.ts223```224225> **Rule:** Edit the file + all dependent files in the SAME task. Never leave broken imports or missing updates.226227---228229## Self-Check Before Completing230231Before marking any task complete, verify:232233| Check | Question |234|-------|----------|235| **Goal met?** | Did I do exactly what was asked? |236| **Files edited?** | Did I modify all necessary files, including dependents? |237| **Code works?** | Did I verify the change compiles and runs? |238| **No errors?** | Do lint and type checks pass? |239| **Nothing forgotten?** | Any edge cases or dependent files missed? |240241---242243## NEVER Do2442451. **NEVER add comments that restate the code** — if the code needs a comment to explain *what* it does, rename things until it doesn't2462. **NEVER create abstractions for fewer than 3 use cases** — premature abstraction is worse than duplication2473. **NEVER leave commented-out code in the codebase** — delete it; version control exists for history2484. **NEVER write functions longer than 20 lines** — extract sub-functions until each does one thing2495. **NEVER nest deeper than 2 levels** — use guard clauses, early returns, or extract functions2506. **NEVER use magic numbers or strings** — define named constants with clear semantics2517. **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 changes2528. **NEVER leave a task with failing lint or type checks** — fix all errors before marking complete253254---255256## References257258Detailed guides for specific clean code topics:259260| Reference | Description |261|-----------|-------------|262| [Anti-Patterns](references/anti-patterns.md) | 21 common mistakes with bad/good code examples across naming, functions, structure, and comments |263| [Code Smells](references/code-smells.md) | Classic code smells catalog with detection patterns — Bloaters, OO Abusers, Change Preventers, Dispensables, Couplers |264| [Refactoring Catalog](references/refactoring-catalog.md) | Essential refactoring patterns with before/after examples and step-by-step mechanics |