Back to Details

code-review

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Code Review Best Practices

代码审查最佳实践

Core Principles

核心原则

Review Mindset:
  • Be constructive, explain the "why"
  • Prioritize by severity (critical vs nice-to-have)
  • Suggest alternatives, not just problems
  • Acknowledge good work
Goals:
  1. Catch bugs before production
  2. Improve code quality
  3. Share knowledge
  4. Prevent security vulnerabilities
  5. Ensure consistency
审查心态：
  • 保持建设性，解释“原因”
  • 按严重程度优先处理（关键问题 vs 锦上添花的优化）
  • 提出替代方案，而非仅指出问题
  • 认可优秀的代码实现
目标：
  1. 在代码上线前捕获漏洞
  2. 提升代码质量
  3. 共享知识经验
  4. 防范安全隐患
  5. 确保代码一致性

Security Review

安全审查

OWASP Top 10 Critical Checks

OWASP Top 10 关键检查项

Injection Attacks:
  • SQL: Use parameterized queries, never concatenate user input
  • Command: Avoid shell commands with user input, sanitize properly
  • Code: Validate all eval(), exec(), dynamic execution
  • NoSQL/LDAP/XML: Use safe APIs
Authentication & Authorization:
  • Verify auth checks on protected endpoints
  • Proper session management (timeout, secure cookies)
  • Authorization logic prevents privilege escalation
  • Password policies enforced
Sensitive Data:
  • No hardcoded secrets (API keys, passwords, tokens)
  • Encryption at rest and in transit
  • PII/PHI minimally logged
  • No sensitive data in URLs or error messages
XSS Prevention:
  • All user input escaped/sanitized for output context
  • Content-Security-Policy headers configured
  • Framework built-in escaping (React JSX, template engines)
Deserialization:
  • Never deserialize untrusted data without validation
  • Prefer JSON over pickle/marshal
  • Validate object types post-deserialization
Misconfiguration:
  • No default credentials in production
  • Error messages don't leak internals
  • Security headers present (HSTS, X-Frame-Options)
API Security:
  • Rate limiting on sensitive endpoints
  • CORS properly configured
  • Input validation on all endpoints
  • No sensitive data in GET requests
注入攻击：
  • SQL：使用参数化查询，绝不要拼接用户输入
  • 命令注入：避免使用包含用户输入的Shell命令，需正确做数据清洗
  • 代码注入：验证所有eval()、exec()及动态执行逻辑
  • NoSQL/LDAP/XML：使用安全API
身份认证与授权：
  • 验证受保护端点的认证检查逻辑
  • 合理的会话管理（超时设置、安全Cookie）
  • 授权逻辑需防止权限提升
  • 强制执行密码策略
敏感数据：
  • 禁止硬编码密钥（API密钥、密码、令牌）
  • 静态和传输中的数据均需加密
  • 最小化PII/PHI数据的日志记录
  • 禁止在URL或错误信息中包含敏感数据
XSS防护：
  • 所有用户输入需根据输出场景做转义/清洗
  • 配置Content-Security-Policy头部
  • 利用框架内置的转义机制（React JSX、模板引擎）
反序列化：
  • 未验证的不可信数据绝不能反序列化
  • 优先使用JSON而非pickle/marshal
  • 反序列化后验证对象类型
配置错误：
  • 生产环境禁止使用默认凭据
  • 错误信息不得泄露内部细节
  • 配置必要的安全头部（HSTS、X-Frame-Options）
API安全：
  • 敏感端点需设置速率限制
  • 正确配置CORS
  • 所有端点均需做输入验证
  • GET请求中不得包含敏感数据

Common Security Issues

常见安全问题

  • Path Traversal: Validate paths, prevent 
    ../
    attacks
  • SSRF: Validate URLs, restrict internal network access
  • Open Redirects: Whitelist redirect destinations
  • Race Conditions: Check TOCTOU bugs
  • Timing Attacks: Constant-time comparison for secrets
  • Regex DoS: Avoid complex regex on user input
  • 路径遍历： 验证路径，防范
    ../
    攻击
  • SSRF： 验证URL，限制内部网络访问
  • 开放重定向： 白名单允许的重定向目标
  • 竞态条件： 检查TOCTOU漏洞
  • 时序攻击： 对敏感信息使用固定时间比较
  • 正则表达式DoS： 避免对用户输入使用复杂正则

Code Quality

代码质量

Readability & Maintainability

可读性与可维护性

Naming:
  • Clear, descriptive names that reveal intent
  • Consistent conventions (camelCase, snake_case, PascalCase)
  • Avoid abbreviations unless domain-standard
Function Design:
  • Single responsibility, small (<50 lines ideal)
  • Clear input/output, minimal side effects
  • Pure functions where possible
Complexity:
  • Low cyclomatic complexity (<10)
  • Avoid deep nesting (max 3-4 levels)
  • Early returns to reduce nesting
  • Extract complex conditions into named functions
Documentation:
  • Comments explain "why", not "what"
  • Public APIs documented
  • No commented-out code (use version control)
命名规范：
  • 清晰、表意明确的命名，体现代码意图
  • 一致的命名约定（camelCase、snake_case、PascalCase）
  • 除非是领域通用缩写，否则避免使用缩写
函数设计：
  • 单一职责，函数体量精简（理想情况下少于50行）
  • 输入输出清晰，副作用最小化
  • 尽可能使用纯函数
复杂度控制：
  • 低圈复杂度（<10）
  • 避免深层嵌套（最多3-4层）
  • 提前返回以减少嵌套层级
  • 将复杂条件提取为命名函数
文档规范：
  • 注释解释“为什么”，而非“做什么”
  • 公开API必须有文档
  • 禁止保留注释掉的代码（使用版本控制管理历史代码）

Error Handling

错误处理

  • All errors handled (catch, log, recover or fail fast)
  • No bare except/catch without handling
  • Informative error messages (what, why, action)
  • Proper error types (not generic Exception)
  • Resources cleaned up (finally/defer/using)
Anti-patterns:
  • Swallowing exceptions silently
  • Catching too broadly
  • Using exceptions for flow control
  • Returning null instead of error
  • Not validating inputs
  • 所有错误均需处理（捕获、日志记录、恢复或快速失败）
  • 禁止使用无处理逻辑的bare except/catch
  • 错误信息需具备信息量（包含问题、原因、解决建议）
  • 使用合适的错误类型（而非通用Exception）
  • 资源需正确清理（finally/defer/using语句）
反模式：
  • 静默吞掉异常
  • 捕获范围过宽
  • 使用异常控制流程
  • 返回null而非错误对象
  • 未做输入验证

Code Duplication

代码重复

  • No copy-paste code blocks
  • Extract common logic to reusable functions
  • Use inheritance/composition appropriately
  • Be pragmatic: 3+ copies = refactor time
  • 禁止复制粘贴代码块
  • 将通用逻辑提取为可复用函数
  • 合理使用继承/组合
  • 务实原则：重复3次及以上需重构

Performance

性能优化

Algorithm & Data Structure Efficiency

算法与数据结构效率

  • Check time complexity (O(n²) → O(n log n) or O(n))
  • Appropriate space complexity
  • Right data structure (HashMap vs Array, Set vs List)
  • Efficient algorithms for common problems
  • 检查时间复杂度（O(n²) → O(n log n) 或 O(n)）
  • 合理的空间复杂度
  • 选择合适的数据结构（HashMap vs Array、Set vs List）
  • 针对常见问题使用高效算法

Common Issues

常见问题

Database:
  • N+1 query problems (use joins/batch)
  • Missing indexes on filtered/sorted columns
  • SELECT * instead of specific columns
  • Queries inside loops
Caching:
  • Repeated expensive calculations
  • Duplicate API calls
  • Static data not cached
  • Appropriate cache TTL
Resource Management:
  • Files/connections/streams closed
  • No memory leaks (circular refs, event listeners)
  • Unbounded collections (need limits/pagination)
Frontend:
  • Unnecessary re-renders (React useMemo, useCallback)
  • Large bundles (code splitting)
  • Images not optimized
  • Blocking JavaScript in critical path
数据库：
  • N+1查询问题（使用关联查询/批量操作）
  • 过滤/排序列缺少索引
  • 使用SELECT *而非指定列
  • 在循环中执行查询
缓存：
  • 重复执行的昂贵计算
  • 重复的API调用
  • 静态数据未做缓存
  • 合理设置缓存TTL
资源管理：
  • 文件/连接/流需正确关闭
  • 无内存泄漏（循环引用、事件监听器未移除）
  • 无界集合需设置限制/分页
前端：
  • 不必要的重渲染（使用React useMemo、useCallback）
  • 过大的包体积（代码分割）
  • 图片未优化
  • 关键路径中的阻塞式JavaScript

Architecture & Design

架构与设计

SOLID Principles

SOLID原则

  • Single Responsibility: One reason to change
  • Open/Closed: Open for extension, closed for modification
  • Liskov Substitution: Subtypes substitutable for base types
  • Interface Segregation: Small, focused interfaces
  • Dependency Inversion: Depend on abstractions
  • 单一职责： 一个类/函数只有一个修改理由
  • 开闭原则： 对扩展开放，对修改关闭
  • 里氏替换： 子类可替换父类使用
  • 接口隔离： 小而专注的接口
  • 依赖反转： 依赖抽象而非具体实现

Other Key Principles

其他关键原则

  • DRY: Don't Repeat Yourself
  • KISS: Keep It Simple
  • YAGNI: Don't over-engineer
  • Separation of Concerns: Distinct responsibilities
  • Composition over Inheritance
  • DRY： 不要重复自己
  • KISS： 保持简单
  • YAGNI： 不要过度设计
  • 关注点分离： 职责清晰划分
  • 组合优于继承

Code Structure

代码结构

  • Proper layer separation (presentation, business, data)
  • Dependencies flow one direction
  • No circular dependencies
  • Modules cohesive and loosely coupled
  • Configuration separated from code
  • 合理的分层（展示层、业务层、数据层）
  • 依赖单向流动
  • 无循环依赖
  • 模块高内聚、低耦合
  • 配置与代码分离

Testing

测试

Coverage & Quality

覆盖率与质量

  • Critical paths tested
  • Edge cases covered (empty, null, max values)
  • Error paths tested
  • Tests deterministic (no flaky tests)
  • Tests isolated (no shared state)
Test Quality:
  • Arrange-Act-Assert structure
  • One assertion focus per test
  • Descriptive test names
  • No test logic (tests are simple)
  • Realistic test data
  • External dependencies mocked appropriately
  • 关键路径需覆盖测试
  • 覆盖边缘场景（空值、null、最大值）
  • 错误路径需覆盖测试
  • 测试需确定（无不稳定测试）
  • 测试需隔离（无共享状态）
测试质量：
  • 遵循Arrange-Act-Assert结构
  • 每个测试聚焦一个断言
  • 测试名称表意明确
  • 测试逻辑简单（无复杂逻辑）
  • 测试数据真实合理
  • 外部依赖需合理Mock

Review Process

审查流程

Before Review

审查前准备

  1. Understand context (PR description, tickets)
  2. Check scope (< 400 lines ideal)
  3. Run code locally
  4. Verify CI passes
  1. 了解上下文（PR描述、需求工单）
  2. 检查代码范围（理想情况下少于400行）
  3. 本地运行代码
  4. 验证CI已通过

During Review

审查中

  1. Start with architecture/approach
  2. Use security/performance/quality checklists
  3. Review tests first (explain intended behavior)
  4. Ask questions, don't assume
  1. 先从架构/实现思路入手
  2. 使用安全/性能/质量检查清单
  3. 先审查测试代码（明确预期行为）
  4. 提出问题，而非主观假设

Providing Feedback

提供反馈

Structure:
  • Severity: Critical (must fix) vs Nice-to-have
  • What: Specific issue
  • Why: Why it matters
  • How: Concrete alternative
Tone:
  • Use "we" language: "We should..." not "You should..."
  • Ask questions: "Have we considered...?"
  • Be specific: "Function has complexity 32, consider refactoring"
Example: ❌ "This is bad" ✅ "Using string concatenation in loop creates O(n²) complexity. Use StringBuilder for O(n)."
结构：
  • 严重程度：关键（必须修复）vs 锦上添花
  • 问题：具体的问题点
  • 原因：问题的影响
  • 方案：具体的替代方案
语气：
  • 使用“我们”的表述：“我们应该...”而非“你应该...”
  • 以提问方式：“我们是否考虑过...？”
  • 具体明确：“该函数圈复杂度为32，建议重构”
示例： ❌ “这写得很差” ✅ “在循环中使用字符串拼接会导致O(n²)的时间复杂度，建议使用StringBuilder实现O(n)复杂度。”

Priority Checklist

优先级检查清单

Critical (Must Fix)

关键（必须修复）

  • Security vulnerabilities (injection, XSS, auth bypass)
  • Data loss/corruption risks
  • Memory/resource leaks
  • Breaking API changes without versioning
  • Race conditions/concurrency bugs
  • 安全漏洞（注入、XSS、认证绕过）
  • 数据丢失/损坏风险
  • 内存/资源泄漏
  • 未做版本管理的破坏性API变更
  • 竞态条件/并发漏洞

High Priority

高优先级

  • Performance issues (O(n²), N+1 queries)
  • Missing input validation
  • Hardcoded configuration
  • Missing error logging
  • No tests for new functionality
  • 性能问题（O(n²)、N+1查询）
  • 缺少输入验证
  • 硬编码配置
  • 缺少错误日志
  • 新功能无测试覆盖

Medium Priority

中优先级

  • Code duplication (3+ instances)
  • High complexity (>15 cyclomatic)
  • Missing documentation for public APIs
  • Deep nesting (>4 levels)
  • Large functions (>100 lines)
  • 代码重复（3次及以上）
  • 高复杂度（圈复杂度>15）
  • 公开API缺少文档
  • 深层嵌套（>4层）
  • 大函数（>100行）

Low Priority

低优先级

  • Minor style inconsistencies
  • Could be more idiomatic
  • Variable names could be clearer
  • Magic numbers → constants
  • 轻微的风格不一致
  • 代码可更符合语言习惯
  • 变量名称可更清晰
  • 魔法数值→常量

Language-Specific Notes

语言特定注意事项

JavaScript/TypeScript:
  • Use const over let, avoid var
  • Proper async/await (handle rejections)
  • Use === not ==
  • TypeScript: Proper types, avoid any
  • Null/undefined handling
Python:
  • Follow PEP 8
  • Type hints for public APIs
  • Context managers (with)
  • No mutable default arguments
Java:
  • Proper exception hierarchy
  • Try-with-resources
  • Access modifiers (private/protected/public)
  • Immutability (final fields)
Go:
  • Error handling on every call
  • defer for cleanup
  • No goroutine leaks (context cancellation)
  • Small, focused interfaces
JavaScript/TypeScript：
  • 优先使用const而非let，避免使用var
  • 正确使用async/await（处理rejections）
  • 使用===而非==
  • TypeScript：正确定义类型，避免使用any
  • 处理Null/undefined
Python：
  • 遵循PEP 8规范
  • 公开API添加类型提示
  • 使用上下文管理器（with）
  • 避免可变默认参数
Java：
  • 合理的异常层级
  • 使用Try-with-resources
  • 正确使用访问修饰符（private/protected/public）
  • 不可变性（final字段）
Go：
  • 每个调用都需处理错误
  • 使用defer做资源清理
  • 无goroutine泄漏（上下文取消）
  • 小而专注的接口

Best Practices Summary

最佳实践总结

  1. Security First: OWASP Top 10 vulnerabilities
  2. Test Coverage: Meaningful tests exist
  3. Error Handling: All errors handled
  4. Performance: Watch O(n²), N+1, memory leaks
  5. Readability: Clear, self-documenting code
  6. Maintainability: Low complexity, no duplication
  7. Documentation: Public APIs documented
  8. Consistency: Follow team conventions
  9. Constructive: Be helpful, not critical
  10. Prioritize: Critical first, style last
  1. 安全优先：关注OWASP Top 10漏洞
  2. 测试覆盖：确保关键路径有意义的测试
  3. 错误处理：所有错误均需妥善处理
  4. 性能优化：警惕O(n²)复杂度、N+1查询、内存泄漏
  5. 可读性：清晰、自文档化的代码
  6. 可维护性：低复杂度、无重复代码
  7. 文档规范：公开API需有文档
  8. 一致性：遵循团队规范
  9. 建设性：提供帮助而非批评
  10. 优先级：先处理关键问题，最后关注风格细节