Clean Code Reviewer

Clean Code Review

Grounded in the principles of Clean Code (Robert C. Martin), focused on 7 high-leverage check dimensions.

Workflow

Review Progress:
- [ ] 1. Scan codebase: identify files to review (default to recently changed code if scope is unspecified)
- [ ] 2. Check each dimension (naming, functions, DRY, YAGNI, magic numbers, clarity, conventions)
- [ ] 3. Rate severity (高/中/低) for each issue
- [ ] 4. Generate report sorted by severity (highest first)

Severity reflects maintainability impact, so the report leads with what to fix first. Prefer the few highest-leverage findings over an exhaustive list of 低 smells — a signal-dense report the human acts on beats a long one they ignore.

When the codebase is primarily Python or Go, consult references/language-patterns.md for language-specific smells before finalizing.

Core Principle: Behavior Preservation

Every suggestion targets only how the code is implemented — never suggest changing the code's functionality, output, or behavior.

Check Dimensions

These are the detection signals and thresholds — the load-bearing decision criteria. Full ❌/✅ worked examples for dimensions 1–5 live in references/detailed-examples.md; read it when you need richer cases or are unsure a finding qualifies.

1. Naming Problems (Meaningful Names)

Detection signals:

• Meaningless names like data1, temp, result, info, obj
• Multiple names for the same concept (mixing get/fetch/retrieve)
• Booleans missing an is/has/can/should prefix

const data1 = fetchUser();   // ❌  →  const userProfile = fetchUser();  // ✅

2. Function Problems (Small Functions + SRP)

Detection signals:

• Function exceeds 100 lines
• More than 3 parameters (use a parameter object instead)
• Function does multiple things (violates Single Responsibility)
• Function name implies read-only but it has side effects

3. Duplication (DRY)

Detection signals:

• Similar if-else structures
• Similar data-transformation / error-handling logic
• Copy-paste traces

4. Over-Engineering (YAGNI)

Detection signals:

• if (config.legacyMode) branches that are never true (dead code)
• Interfaces with only one implementation
• Over-defensive / useless try-catch or if-else

5. Magic Numbers (Avoid Hardcoding)

Detection signals:

• Bare numbers with no explanation (retryCount > 3, setTimeout(fn, 86400000))
• Hardcoded strings, status codes, time constants

if (retryCount > 3) {}   // ❌  →  const MAX_RETRY_COUNT = 3; if (retryCount > MAX_RETRY_COUNT) {}  // ✅

6. Structural Clarity (Readability First)

Detection signals:

• Nested ternary operators
• Overly compact one-liners
• Deep conditional nesting (> 3 levels) — prefer guard clauses with early returns

7. Project Conventions (Consistency)

Detection signals:

• Disordered import order (external libraries vs internal modules)
• Inconsistent function declaration style
• Inconsistent naming conventions (mixing camelCase and snake_case)

[!TIP]

Source project conventions from the project root CLAUDE.md / AGENTS.md, plus linter configs (.eslintrc, .prettierrc, ruff/flake8 config).

Severity Levels

Use 高 / 中 / 低 as the literal severity labels in the report — they are part of the output contract.

Output

Emit a Summary first, then P-numbered findings sorted by severity, then patterns worth keeping and any tests needed to refactor safely. Skeleton (read references/output-template.md before finalizing — it is the full, canonical shape):

# Clean Code Review: <scope>

## Summary
<the highest-leverage maintainability risk, one paragraph>

## Findings

### P1: <issue title>
- **原则**: <命名 / 单一职责 / DRY / YAGNI / 魔法数字 / 结构清晰度 / 项目规范>
- **位置**: `<file>:<line>`
- **级别**: 高 / 中 / 低
- **问题**: <what makes the code harder to read, change, or test>
- **建议**: <behavior-preserving refactor direction>
- **Why now**: <risk if left as-is>

## Good Patterns To Keep
- <implementation choice worth preserving>

## Test Gaps
- <tests needed to protect behavior during the refactor>

References

• references/output-template.md — the full canonical report shape; read before finalizing output.
• references/detailed-examples.md — full ❌/✅ worked cases for the 5 core dimensions (naming, functions, DRY, YAGNI, magic numbers); read when you need richer cases or are unsure a finding qualifies.
• references/language-patterns.md — language-specific smells for TypeScript/JavaScript, Python, and Go; consult when the codebase is primarily one of these languages.

Multi-Agent Parallel

When parallelizing across subagents, split the work along one axis, then dedupe and reconcile severity ratings when merging:

1. By check dimension — one agent per dimension (7 total) 2. By module/directory — one agent per module 3. By language — one agent each for TypeScript, Python, Go 4. By file type — components, hooks, utility functions, type definitions

Use a different skill when

This skill reports file/function-level Clean Code findings and does not modify code. Route elsewhere when:

• Architecture / module boundaries / abstraction quality (system-level, APoSD) → hai-architecture.
• Eliminating `any` / TypeScript type safety → ts-type-safety-reviewer.
• Actually applying the refactors (not just reporting) → code-simplifier.
• React component design (consumer API, data flow, testability) → component-diagnosis / react-component-diagnosis.

Detailed Examples by Dimension

Table of Contents

• 1. 命名问题
• 2. 函数问题
• 3. 重复问题
• 4. 过度设计
• 5. 魔法数字

---

1. 命名问题

无意义命名

// ❌ 
const list = getUsers();           // list of what?
const flag = checkPermission();    // what flag?
const handler = () => {};          // handles what?

// ✅ 
const activeUsers = getUsers();
const hasEditPermission = checkPermission();
const onFormSubmit = () => {};

命名不一致

// ❌ 同一概念多种命名
getUserData();
fetchUserInfo();
retrieveUserProfile();

// ✅ 统一使用一种
getUser();
getUserProfile();
getUserSettings();

布尔命名

// ❌ 
const open = true;
const disabled = false;

// ✅ 带 is/has/can/should 前缀
const isOpen = true;
const isDisabled = false;
const hasPermission = true;
const canEdit = true;

---

2. 函数问题

函数过长

// ❌ 160 行的 processOrder 函数
async function processOrder(order) {
  // 验证逻辑 (40 行)
  // 计算逻辑 (30 行)
  // 库存检查 (25 行)
  // 支付处理 (35 行)
  // 通知发送 (30 行)
}

// ✅ 拆分为单一职责函数
async function processOrder(order) {
  await validateOrder(order);
  const total = calculateTotal(order);
  await checkInventory(order.items);
  await processPayment(order, total);
  await sendNotifications(order);
}

参数过多

// ❌ 
function createUser(name, email, age, address, phone, role, department, manager) {}

// ✅ 使用配置对象
interface CreateUserParams {
  name: string;
  email: string;
  profile: { age: number; phone: string };
  organization: { role: string; department: string; manager: string };
}
function createUser(params: CreateUserParams) {}

副作用

// ❌ 函数名暗示只读，但有副作用
function getUser(id) {
  const user = db.find(id);
  user.lastAccess = new Date();  // 副作用！
  db.save(user);                  // 副作用！
  return user;
}

// ✅ 分离读写
function getUser(id) {
  return db.find(id);
}

function recordUserAccess(id) {
  const user = db.find(id);
  user.lastAccess = new Date();
  db.save(user);
}

---

3. 重复问题

相似的验证逻辑

// ❌ 重复的验证模式
function validateUser(user) {
  if (!user.name) throw new Error('Name required');
  if (!user.email) throw new Error('Email required');
  if (!user.age) throw new Error('Age required');
}

function validateProduct(product) {
  if (!product.name) throw new Error('Name required');
  if (!product.price) throw new Error('Price required');
  if (!product.sku) throw new Error('SKU required');
}

// ✅ 提取通用验证器
function validateRequired(obj, fields) {
  for (const field of fields) {
    if (!obj[field]) throw new Error(`${field} required`);
  }
}

validateRequired(user, ['name', 'email', 'age']);
validateRequired(product, ['name', 'price', 'sku']);

相似的错误处理

// ❌ 重复的 try-catch 模式
async function fetchUsers() {
  try {
    return await api.get('/users');
  } catch (e) {
    logger.error('Failed to fetch users', e);
    throw new ApiError('Failed to fetch users');
  }
}

async function fetchProducts() {
  try {
    return await api.get('/products');
  } catch (e) {
    logger.error('Failed to fetch products', e);
    throw new ApiError('Failed to fetch products');
  }
}

// ✅ 提取通用包装器
async function apiCall(endpoint, errorMessage) {
  try {
    return await api.get(endpoint);
  } catch (e) {
    logger.error(errorMessage, e);
    throw new ApiError(errorMessage);
  }
}

const users = await apiCall('/users', 'Failed to fetch users');
const products = await apiCall('/products', 'Failed to fetch products');

---

4. 过度设计

无用的抽象层

// ❌ 只有一个实现的接口
interface IUserRepository {
  findById(id: string): User;
}

class UserRepository implements IUserRepository {
  findById(id: string): User { /* ... */ }
}

// ✅ 直接使用类，需要时再抽象
class UserRepository {
  findById(id: string): User { /* ... */ }
}

过度防御

// ❌ 过度防御的代码
function add(a, b) {
  if (typeof a !== 'number') throw new Error('a must be number');
  if (typeof b !== 'number') throw new Error('b must be number');
  if (isNaN(a)) throw new Error('a is NaN');
  if (isNaN(b)) throw new Error('b is NaN');
  if (!isFinite(a)) throw new Error('a is not finite');
  if (!isFinite(b)) throw new Error('b is not finite');
  return a + b;
}

// ✅ 合理的类型安全 (TypeScript)
function add(a: number, b: number): number {
  return a + b;
}

从未使用的配置

// ❌ 
if (config.enableNewFeature) {  // 一直是 true
  newFeature();
} else {
  oldFeature();  // 死代码
}

// ✅ 删除死代码
newFeature();

---

5. 魔法数字

业务逻辑中的数字

// ❌ 
if (user.age >= 18) {}
if (order.total > 100) {}
if (retryCount < 3) {}

// ✅ 
const LEGAL_AGE = 18;
const FREE_SHIPPING_THRESHOLD = 100;
const MAX_RETRY_ATTEMPTS = 3;

if (user.age >= LEGAL_AGE) {}
if (order.total > FREE_SHIPPING_THRESHOLD) {}
if (retryCount < MAX_RETRY_ATTEMPTS) {}

时间常量

// ❌ 
setTimeout(fn, 86400000);  // 这是多久？
setInterval(poll, 300000); // 这是多久？

// ✅ 
const ONE_DAY_MS = 24 * 60 * 60 * 1000;
const FIVE_MINUTES_MS = 5 * 60 * 1000;

setTimeout(fn, ONE_DAY_MS);
setInterval(poll, FIVE_MINUTES_MS);

HTTP 状态码

// ❌ 
if (response.status === 200) {}
if (response.status === 404) {}

// ✅ 
const HTTP_OK = 200;
const HTTP_NOT_FOUND = 404;
// 或使用常量库: import { StatusCodes } from 'http-status-codes';

Language-Specific Patterns

Table of Contents

• TypeScript/JavaScript
• Python
• Go

---

TypeScript/JavaScript

any 类型滥用

// ❌ 
function process(data: any) {
  return data.value;
}

// ✅ 
interface DataPayload {
  value: string;
}
function process(data: DataPayload) {
  return data.value;
}

回调地狱

// ❌ 
getUser(id, (user) => {
  getOrders(user.id, (orders) => {
    processOrders(orders, (result) => {
      sendNotification(result, () => {
        console.log('done');
      });
    });
  });
});

// ✅ 
const user = await getUser(id);
const orders = await getOrders(user.id);
const result = await processOrders(orders);
await sendNotification(result);

可选链缺失

// ❌ 
if (user && user.profile && user.profile.address && user.profile.address.city) {}

// ✅ 
if (user?.profile?.address?.city) {}

解构赋值

// ❌ 
const name = user.name;
const email = user.email;
const age = user.age;

// ✅ 
const { name, email, age } = user;

---

Python

列表推导滥用

# ❌ 过于复杂的列表推导
result = [x.value for x in items if x.is_valid and x.type == 'A' for y in x.children if y.active]

# ✅ 拆分为函数
def get_active_children(items):
    for item in items:
        if item.is_valid and item.type == 'A':
            for child in item.children:
                if child.active:
                    yield child.value

result = list(get_active_children(items))

可变默认参数

# ❌ 危险！可变对象作为默认参数
def add_item(item, items=[]):
    items.append(item)
    return items

# ✅ 
def add_item(item, items=None):
    if items is None:
        items = []
    items.append(item)
    return items

裸 except

# ❌ 
try:
    risky_operation()
except:
    pass

# ✅ 
try:
    risky_operation()
except SpecificError as e:
    logger.warning(f"Operation failed: {e}")

字符串拼接

# ❌ 
message = "Hello " + name + "! You have " + str(count) + " messages."

# ✅ 
message = f"Hello {name}! You have {count} messages."

类型提示缺失

# ❌ 
def process(data):
    return data['value']

# ✅ 
from typing import TypedDict

class DataPayload(TypedDict):
    value: str

def process(data: DataPayload) -> str:
    return data['value']

---

Go

忽略错误

// ❌ 
result, _ := someFunction()

// ✅ 
result, err := someFunction()
if err != nil {
    return fmt.Errorf("someFunction failed: %w", err)
}

过长的 init 函数

// ❌ init() 做太多事
func init() {
    // 数据库连接
    // 配置加载
    // 缓存初始化
    // 日志设置
    // 100+ 行...
}

// ✅ 拆分职责
func init() {
    initConfig()
    initLogger()
}

func main() {
    db := initDatabase()
    cache := initCache()
    // ...
}

空接口滥用

// ❌ 
func process(data interface{}) {
    v := data.(map[string]interface{})
    // ...
}

// ✅ 
type Payload struct {
    Value string `json:"value"`
}

func process(data Payload) {
    // 类型安全
}

过深嵌套

// ❌ 
func process(order *Order) error {
    if order != nil {
        if order.Items != nil {
            if len(order.Items) > 0 {
                for _, item := range order.Items {
                    if item.Valid {
                        // 实际逻辑
                    }
                }
            }
        }
    }
    return nil
}

// ✅ 早返回 (Guard Clauses)
func process(order *Order) error {
    if order == nil {
        return nil
    }
    if order.Items == nil || len(order.Items) == 0 {
        return nil
    }
    for _, item := range order.Items {
        if !item.Valid {
            continue
        }
        // 实际逻辑
    }
    return nil
}

context 滥用

// ❌ 用 context 传业务数据
ctx = context.WithValue(ctx, "user", user)
ctx = context.WithValue(ctx, "order", order)

// ✅ context 只用于取消和超时，业务数据显式传递
func processOrder(ctx context.Context, user User, order Order) error {
    // ...
}

Clean Code Review Output Template

The canonical report shape. Findings are P-numbered and sorted by severity (highest first). Severity labels are 高 / 中 / 低, matching the rubric in SKILL.md. The behavior-preserving constraint is inherited from SKILL.md — every recommendation stays within it.

# Clean Code Review: <scope>

## Summary
<最高杠杆的可维护性风险，一段话>

## Findings

### P1: <issue title>
- **原则**: <命名 / 单一职责 / DRY / YAGNI / 魔法数字 / 结构清晰度 / 项目规范>
- **位置**: `<file>:<line>`
- **级别**: 高 / 中 / 低
- **问题**: <what makes the code harder to read, change, or test>
- **建议**: <refactor direction>
- **Why now**: <risk if left as-is>

### P2: <issue title>
- **原则**: <principle>
- **位置**: `<file>:<line>`
- **级别**: 高 / 中 / 低
- **问题**: <description>
- **建议**: <direction>
- **Why now**: <risk>

## Good Patterns To Keep
- <specific implementation choice worth preserving>

## Test Gaps
- <only tests needed to protect behavior during refactor>