轩辕李的博客
轩辕李
2022-03-24
目录

代码质量管理之代码审查

代码规范，是团队开发的基础。使用好的工具，会事半功倍。最后加上一道代码审查，才能真正的做到代码质量管理的闭环。

说实话，写了这么多年代码，我发现代码审查这件事儿，真的是既让人爱又让人恨。爱的是它确实能让代码质量上一个台阶，恨的是有时候审查过程真的挺磨人。不过经历了这么多项目后，我越来越觉得，一个好的代码审查机制，对团队来说简直就是救命稻草。

# 一、什么是代码审查

简单来说，代码审查（Code Review）就是让别人帮你看看代码有没有问题。听起来挺简单对吧？但实际操作起来，这里面的学问可大了。

我记得刚工作的时候，第一次被要求做代码审查，心里那叫一个紧张。生怕自己的代码被挑出一堆毛病，更怕审查别人代码的时候看不出问题来。后来才发现，代码审查其实是一个互相学习的过程，不是考试，也不是批斗会。

代码审查主要关注这几个方面：

  • 逻辑正确性：代码能不能正确完成预期功能
  • 性能表现：有没有明显的性能瓶颈
  • 安全性：是否存在安全隐患
  • 可读性：其他人能不能轻松理解你的代码
  • 可维护性：后续修改和扩展是否方便

# 二、为什么要做代码审查

# 1、尽早发现问题

有个数据很有意思：修复一个 bug 的成本，在编码阶段是 1，在测试阶段就变成了 10，到了生产环境可能就是 100 甚至更多。代码审查能在最早期就发现问题，这笔账算下来，真的很划算。

举个我亲身经历的例子：有次一个同事写了个批量处理数据的功能，代码里用了个简单的循环查询数据库。在代码审查时，我们发现这个写法在数据量大的时候会有 N+1 查询问题。如果这个问题到了生产环境才发现，那可就麻烦大了。

# 2、知识共享和团队成长

代码审查是团队内最好的知识共享方式之一。新人可以通过审查学习项目规范和最佳实践，老手也能从新人那里获得一些新的思路和技术。

我们团队有个传统，每周会选一段有代表性的代码，大家一起审查讨论。这个过程中，经常会碰撞出很多有意思的想法。比如有次讨论一个缓存策略的实现，一个刚毕业的小伙伴提出了用布隆过滤器来解决缓存穿透的方案，让我们这些老鸟都眼前一亮。

# 3、保持代码一致性

没有代码审查的项目，时间一长，你会发现代码风格五花八门。有人喜欢用 if-else，有人偏爱三元运算符；有人习惯长函数，有人追求短小精悍。代码审查能让团队保持相对统一的编码风格，这对项目的长期维护非常重要。

# 三、代码审查的实际流程

让我用一个真实的例子来说明代码审查的流程。假设你要提交一个新功能的代码：

# 1、准备阶段

在提交审查之前，先做好这些准备：

# 1. 确保代码能正常运行
npm test

# 2. 运行代码格式化
npm run format

# 3. 检查代码规范
npm run lint

# 4. 自己先过一遍代码
git diff --cached

# 2、创建 Pull Request

创建 PR 时，写清楚这几点：

  • 改动内容：简要说明做了什么
  • 改动原因：为什么要这么改
  • 测试情况：如何测试的
  • 注意事项：有什么需要特别关注的

一个好的 PR 描述示例：

## 概述
修复用户登录时偶发的 session 丢失问题

## 问题原因
Redis 连接池在高并发时会出现连接超时，导致 session 写入失败

## 解决方案
1. 增加 Redis 连接池大小（10 -> 50）
2. 添加重试机制（最多重试 3 次）
3. 增加连接超时监控告警

## 测试
- 单元测试：✅ 全部通过
- 压力测试：1000 并发用户，无 session 丢失
- 监控验证：告警功能正常触发

## 需要关注
- RedisConfig.java 第 45-67 行：连接池配置
- SessionService.java 第 123-156 行：重试逻辑

# 3、审查过程

作为审查者，我通常会按这个顺序来看代码：

  1. 先看测试：通过测试了解功能预期
  2. 看接口设计：API 是否合理，是否向后兼容
  3. 看核心逻辑：算法是否正确，有无边界情况遗漏
  4. 看代码质量：命名、注释、代码结构等
  5. 看性能和安全：是否有明显的性能问题或安全隐患

# 4、提供反馈

提反馈时要注意方式方法。我总结了几个原则：

好的反馈方式：

// 建议：这里使用 StringBuilder 会更高效
// 原因：在循环中使用 + 拼接字符串会创建大量临时对象
String result = "";
for (String item : list) {
    result += item;  // 可以改用 StringBuilder
}

不好的反馈方式：

// 这代码写得太烂了，性能差得要命！

# 四、常用代码审查工具对比

在实际工作中，选择合适的工具能让代码审查事半功倍。下面是我用过的几个主流工具的对比：

# 1、Pull Request

优点：

  • 集成度高，与 GitHub 生态完美融合
  • 界面简洁直观
  • 支持 inline 评论，讨论很方便
  • 免费版功能就很强大

缺点：

  • 对大文件的 diff 展示不太友好
  • 审查规则配置相对简单

适用场景： 开源项目或使用 GitHub 托管的项目

# 2、Merge Request

优点：

  • 功能全面，企业版功能强大
  • 支持自动化审查流程
  • 可以自部署，数据安全性高
  • 内置 CI/CD，集成度好

缺点：

  • 自部署版本维护成本较高
  • 界面相对复杂

适用场景： 需要私有化部署的企业项目

# 3、Gerrit

优点：

  • 审查流程严格，适合大型项目
  • 支持逐个 commit 审查
  • 权限控制细致

缺点：

  • 学习曲线陡峭
  • UI 不够友好
  • 配置复杂

适用场景： 对代码质量要求极高的大型项目

# 4、Bitbucket

优点：

  • 与 Jira 集成很好
  • 支持分支权限管理
  • 企业功能完善

缺点：

  • 免费版限制较多
  • 性能有时不稳定

适用场景： 使用 Atlassian 全家桶的团队

# 五、代码审查清单（Checklist）

为了让审查更高效，我整理了一份实用的审查清单：

# 1、功能性检查

  • [ ] 代码是否实现了需求文档中的所有功能？
  • [ ] 边界条件是否都考虑到了？
  • [ ] 错误处理是否完善？
  • [ ] 是否有合适的日志记录？

# 2、代码质量检查

  • [ ] 命名是否清晰、符合规范？
  • [ ] 函数是否过长（建议不超过 50 行）？
  • [ ] 是否有重复代码可以抽取？
  • [ ] 注释是否准确、必要？

# 3、性能检查

  • [ ] 是否有明显的性能瓶颈？
  • [ ] 数据库查询是否优化（避免 N+1 查询）？
  • [ ] 是否合理使用缓存？
  • [ ] 大数据量场景是否考虑分页或批处理？

# 4、安全性检查

  • [ ] 是否有 SQL 注入风险？
  • [ ] 敏感信息是否加密存储？
  • [ ] 权限验证是否完善？
  • [ ] 是否有 XSS 或 CSRF 风险？

# 5、可维护性检查

  • [ ] 代码结构是否清晰？
  • [ ] 是否遵循项目的架构规范？
  • [ ] 测试覆盖率是否足够？
  • [ ] 文档是否更新？

# 六、常见问题及解决方案

# 1、问题1：审查流于形式

现象： 审查者只是简单看一眼就通过，没有实质性反馈。

解决方案：

  • 设置最少审查人数要求（比如至少 2 人）
  • 定期统计审查质量指标
  • 建立审查者轮换机制
  • 对发现重要问题的审查者给予认可

# 2、问题2：审查周期过长

现象： PR 提交后好几天都没人审查，影响开发进度。

解决方案：

  • 设置审查 SLA（比如 24 小时内必须完成）
  • 控制 PR 大小（建议不超过 400 行）
  • 建立审查优先级机制
  • 使用自动分配审查者的工具

# 3、问题3：审查意见难以达成一致

现象： 审查者之间意见不一，讨论陷入僵局。

解决方案：

  • 制定明确的代码规范
  • 设立技术委员会做最终裁决
  • 区分必须修改和建议修改
  • 定期组织技术分享，统一认知

# 4、问题4：新人不敢提意见

现象： 新人审查老员工代码时不敢指出问题。

解决方案：

  • 营造平等的技术氛围
  • 鼓励提问和讨论
  • 老员工主动征求新人意见
  • 建立 mentor 制度，帮助新人成长

# 七、最佳实践建议

# 1、保持 PR 小而精

大的 PR 很难审查透彻。我的经验是：

  • 功能性改动：不超过 400 行
  • 重构：不超过 800 行
  • 如果必须大改，先拆分成多个 PR

# 2、自动化能自动化的

把能自动化的都自动化，让人专注于真正需要人工判断的部分：

# .github/workflows/pr-check.yml
name: PR Check
on: [pull_request]
jobs:
  check:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - name: Run tests
        run: npm test
      - name: Check code style
        run: npm run lint
      - name: Check test coverage
        run: npm run coverage

# 3、建立良好的审查文化

  • 相互尊重：对事不对人，focus 在代码而不是人
  • 及时响应：收到审查请求后尽快响应
  • 持续学习：把每次审查当作学习机会
  • 适度要求：不要过度追求完美

# 4、使用审查模板

创建 PR 模板可以规范化审查流程：

<!-- .github/pull_request_template.md -->
## 类型
- [ ] Bug 修复
- [ ] 新功能
- [ ] 重构
- [ ] 文档更新

## 改动说明
<!-- 请详细说明改动内容 -->

## 测试情况
- [ ] 单元测试通过
- [ ] 集成测试通过
- [ ] 手工测试通过

## 影响范围
<!-- 列出可能受影响的模块 -->

## 相关 Issue
<!-- 关联的 Issue 编号 -->

# 八、实战案例分享

让我分享一个真实的案例，展示代码审查如何避免了一个严重问题：

# 1、背景

我们的电商系统需要实现一个促销活动功能，计算用户的优惠金额。初版代码是这样的：

public BigDecimal calculateDiscount(Order order, Promotion promotion) {
    BigDecimal discount = BigDecimal.ZERO;
    
    // 计算折扣
    if (promotion.getType() == PromotionType.PERCENTAGE) {
        discount = order.getTotal().multiply(promotion.getValue());
    } else if (promotion.getType() == PromotionType.FIXED) {
        discount = promotion.getValue();
    }
    
    return discount;
}

# 2、审查发现的问题

  1. 精度问题：百分比折扣没有除以 100
  2. 边界检查：折扣可能超过订单总额
  3. null 安全：没有检查空值
  4. 业务逻辑：没有考虑优惠券叠加规则

# 3、改进后的代码

public BigDecimal calculateDiscount(Order order, Promotion promotion) {
    // 参数校验
    if (order == null || promotion == null) {
        return BigDecimal.ZERO;
    }
    
    BigDecimal orderTotal = order.getTotal();
    if (orderTotal == null || orderTotal.compareTo(BigDecimal.ZERO) <= 0) {
        return BigDecimal.ZERO;
    }
    
    BigDecimal discount = BigDecimal.ZERO;
    
    // 根据促销类型计算折扣
    switch (promotion.getType()) {
        case PERCENTAGE:
            // 百分比折扣需要除以 100
            BigDecimal percentage = promotion.getValue().divide(new BigDecimal("100"));
            discount = orderTotal.multiply(percentage);
            break;
        case FIXED:
            discount = promotion.getValue();
            break;
        default:
            log.warn("Unknown promotion type: {}", promotion.getType());
    }
    
    // 确保折扣不超过订单总额
    if (discount.compareTo(orderTotal) > 0) {
        discount = orderTotal;
    }
    
    // 保留两位小数
    return discount.setScale(2, RoundingMode.HALF_UP);
}

这个案例充分说明了代码审查的价值。如果这个 bug 进入生产环境，可能会造成严重的经济损失。

# 九、总结

代码审查不仅仅是找 bug 那么简单，它是提升整个团队代码质量和技术水平的重要手段。通过合理的流程、合适的工具、良好的文化，代码审查能够：

  1. 提前发现问题，降低修复成本
  2. 促进知识共享，提升团队整体水平
  3. 保证代码质量，提高项目可维护性
  4. 培养工程文化，建立技术规范

记住，代码审查的目的不是挑毛病，而是一起把代码写得更好。保持开放的心态，无论是提意见还是接受意见，都要记得我们的共同目标是让项目更成功。

最后，送大家一句话：好的代码审查，是让代码变得更好，也让写代码的人变得更好。

祝你变得更强！

编辑 (opens new window)
上次更新: 2025/08/15
代码质量管理之工具篇
Git提交规范

← Git提交规范→

最近更新
01
AI时代的编程心得
09-11
02
Claude Code与Codex的协同工作
09-01
03
Claude Code实战之供应商切换工具
08-18
更多文章>
Theme by Vdoing | Copyright © 2018-2025 京ICP备2021021832号-2 | MIT License