67-代码审查实践详解

代码审查实践详解

一、知识概述

代码审查（Code Review）是软件开发中保证代码质量的重要环节，通过同行评审发现代码缺陷、提升代码质量、促进知识共享。有效的代码审查不仅能发现Bug，还能提升团队整体技术水平。

本文将深入分析代码审查的最佳实践，包括审查流程、审查要点、常见问题模式、工具推荐等内容。

代码审查的核心价值

1. 缺陷发现：早期发现Bug，降低修复成本
2. 知识共享：团队成员互相学习
3. 质量保证：确保代码符合规范
4. 团队协作：促进沟通，打破孤岛

---

二、知识点详细讲解

2.1 代码审查流程

graph LR
    A[提交代码] --> B[创建Pull Request]
    B --> C[自动检查]
    C --> D{通过?}
    D -->|否| E[修复问题]
    E --> A
    D -->|是| F[人工审查]
    F --> G{批准?}
    G -->|否| H[提出修改建议]
    H --> I[修改代码]
    I --> F
    G -->|是| J[合并代码]

2.2 审查清单

功能正确性

/**
 * 审查要点：功能是否正确实现
 */

// ✗ 问题：边界条件处理不当
public int divide(int a, int b) {
    return a / b;  // 未处理b=0的情况
}

// ✓ 正确
public int divide(int a, int b) {
    if (b == 0) {
        throw new IllegalArgumentException("除数不能为0");
    }
    return a / b;
}

// ✗ 问题：空指针风险
public String getUserName(User user) {
    return user.getName();  // user可能为null
}

// ✓ 正确
public String getUserName(User user) {
    if (user == null) {
        return "Unknown";
    }
    return user.getName();
}

// ✗ 问题：并发安全问题
public class Counter {
    private int count = 0;
    
    public void increment() {
        count++;  // 非原子操作
    }
}

// ✓ 正确
public class Counter {
    private AtomicInteger count = new AtomicInteger(0);
    
    public void increment() {
        count.incrementAndGet();
    }
}

代码质量

/**
 * 审查要点：代码可读性、可维护性
 */

// ✗ 问题：命名不清晰
public int calc(int a, int b) {
    return a * b + 10;
}

// ✓ 正确
public int calculateTotalPrice(int quantity, int unitPrice) {
    final int SHIPPING_FEE = 10;
    return quantity * unitPrice + SHIPPING_FEE;
}

// ✗ 问题：方法过长（超过80行）
public void processOrder(Order order) {
    // 100+ 行代码...
}

// ✓ 正确：拆分为多个方法
public void processOrder(Order order) {
    validateOrder(order);
    calculatePrice(order);
    deductStock(order);
    createPayment(order);
    sendNotification(order);
}

// ✗ 问题：重复代码
public double calculateCircleArea(double radius) {
    return Math.PI * radius * radius;
}

public double calculateCylinderVolume(double radius, double height) {
    return Math.PI * radius * radius * height;  // 重复计算圆面积
}

// ✓ 正确：复用代码
public double calculateCircleArea(double radius) {
    return Math.PI * radius * radius;
}

public double calculateCylinderVolume(double radius, double height) {
    return calculateCircleArea(radius) * height;
}

性能问题

/**
 * 审查要点：性能优化
 */

// ✗ 问题：循环内数据库查询
public List<OrderVO> getOrders(List<Long> orderIds) {
    List<OrderVO> result = new ArrayList<>();
    for (Long orderId : orderIds) {
        result.add(orderDao.findById(orderId));  // N+1问题
    }
    return result;
}

// ✓ 正确：批量查询
public List<OrderVO> getOrders(List<Long> orderIds) {
    return orderDao.findByIds(orderIds);
}

// ✗ 问题：字符串拼接
public String buildMessage(List<String> items) {
    String result = "";
    for (String item : items) {
        result += item + ",";  // 每次创建新String对象
    }
    return result;
}

// ✓ 正确：使用StringBuilder
public String buildMessage(List<String> items) {
    StringBuilder sb = new StringBuilder();
    for (String item : items) {
        sb.append(item).append(",");
    }
    return sb.toString();
}

// ✗ 问题：集合未指定大小
public void processItems(List<String> items) {
    List<String> result = new ArrayList<>();  // 可能多次扩容
    for (String item : items) {
        result.add(transform(item));
    }
}

// ✓ 正确：预估容量
public void processItems(List<String> items) {
    List<String> result = new ArrayList<>(items.size());
    for (String item : items) {
        result.add(transform(item));
    }
}

安全问题

/**
 * 审查要点：安全漏洞
 */

// ✗ 问题：SQL注入风险
public User findUser(String username) {
    String sql = "SELECT * FROM user WHERE username = '" + username + "'";
    return jdbcTemplate.queryForObject(sql, User.class);
}

// ✓ 正确：使用参数化查询
public User findUser(String username) {
    String sql = "SELECT * FROM user WHERE username = ?";
    return jdbcTemplate.queryForObject(sql, User.class, username);
}

// ✗ 问题：敏感信息日志
public void login(String username, String password) {
    log.info("User login: {}, password: {}", username, password);  // 泄露密码
}

// ✓ 正确：脱敏处理
public void login(String username, String password) {
    log.info("User login: {}", username);
}

// ✗ 问题：文件路径未校验
public void readFile(String filename) throws IOException {
    Files.readAllLines(Paths.get(filename));  // 可能访问任意文件
}

// ✓ 正确：限制访问目录
public void readFile(String filename) throws IOException {
    Path basePath = Paths.get("/data/files");
    Path filePath = basePath.resolve(filename).normalize();
    
    if (!filePath.startsWith(basePath)) {
        throw new SecurityException("非法文件路径");
    }
    
    Files.readAllLines(filePath);
}

2.3 审查工具

GitLab CI配置

# .gitlab-ci.yml
stages:
  - build
  - test
  - review

build:
  stage: build
  script:
    - mvn compile

test:
  stage: test
  script:
    - mvn test
  coverage: '/Total.*?([0-9]{1,3})%/'

sonarqube-check:
  stage: review
  script:
    - mvn sonar:sonar
  allow_failure: false
  only:
    - merge_requests

GitHub Actions配置

# .github/workflows/code-review.yml
name: Code Review

on: [pull_request]

jobs:
  review:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      
      - name: Set up JDK 17
        uses: actions/setup-java@v4
        with:
          java-version: '17'
      
      - name: Build
        run: mvn compile
      
      - name: Test
        run: mvn test
      
      - name: SonarCloud Scan
        uses: SonarSource/sonarcloud-github-action@master
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}

---

三、总结与最佳实践

3.1 审查原则

1. 小批量：每次审查不超过400行代码
2. 及时性：24小时内完成审查
3. 建设性：关注代码，不针对个人
4. 尊重性：礼貌表达意见

3.2 审查流程

1. 自动化检查先行
2. 人工审查核心逻辑
3. 提出修改建议
4. 验证修改结果
5. 批准合并

3.3 常见问题模式

类型	常见问题	解决方案
功能	边界条件	添加边界检查
性能	N+1查询	批量查询
安全	SQL注入	参数化查询
可读性	命名不清	重命名

---

参考资料

1. 《代码大全》
2. Google Engineering Practices
3. 《Clean Code》
4. GitLab Code Review Guidelines

---

六、思考与练习

思考题

1. 基础题：代码审查的核心价值有哪些？为什么说"审查要关注代码，不针对个人"？
2. 进阶题：N+1查询问题是什么？如何在代码审查中发现并避免这类问题？
3. 实战题：如何平衡代码审查的严格程度和开发效率？审查的粒度应该如何控制？

编程练习

练习：为一个Pull Request进行完整的代码审查，使用审查清单逐项检查，提出至少5条有建设性的修改建议，包括功能、性能、安全等方面。

章节关联

• 前置章节：代码规范详解
• 后续章节：持续集成详解
• 扩展阅读：《代码大全》、Google Engineering Practices、《Clean Code》

---

📝 下一章预告

持续集成是现代软件开发的基石。下一章将系统讲解Jenkins Pipeline、GitLab CI、GitHub Actions三大主流工具的使用方法，帮助你构建自动化CI/CD流水线。

---

本章完