clean-code-review

name	clean-code-review
description	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.

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');

   (order);
}

Parameter Objects

When a function needs more than 3 arguments, use an options object.

// BAD — too many parameters, order matters
createUser('John', 'Doe', '[e-mail]', 'secret', 'admin', 'Engineering');

// GOOD — self-documenting options object
createUser({
  firstName: 'John',
  lastName: 'Doe',
  email: '[e-mail]',
  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

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	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