我为什么把“代码评审”从走过场变成了救命绳

刚入行那几年，我对代码评审（Code Review）的态度很敷衍：不就是走个流程吗？扫一眼，写句“LGTM”（Looks Good To Me），赶紧合并，别耽误上线。直到我亲手埋下的一个 Bug 在生产环境炸了，才彻底明白：评审不是形式主义，是团队给你上的最后一道保险。

那次事故很简单，也很蠢。我写了一个“批量更新订单状态”的接口，SQL 大概是：

UPDATE order SET status = #{newStatus} WHERE user_id = #{userId}

看起来没问题对吧？但我在 MyBatis 的 XML 里漏写了 user_id 参数的传入，结果这行 SQL 实际执行时变成了：

UPDATE order SET status = 1

没有 WHERE 条件，全表更新。 好在 DBA 反应快，3 秒内执行了 KILL 命令，但仍有几千条订单状态被刷成了“已完成”。那天下午，我一边写事故报告，一边想：如果当时有人认真看了这段代码，哪怕只是问一句“这个更新有 WHERE 条件吗”，这一切都不会发生。

从那以后，我彻底变了个人。现在我做评审时，会刻意盯住几个“高危区”：

1. 数据写操作：UPDATE / DELETE 必须有 WHERE，且必须有安全护栏

我现在看到任何更新或删除语句，第一反应就是找 WHERE 条件。如果 WHERE 里用的是 IN 子查询，我会追问：子查询返回空时会不会变成全表操作？如果参数是集合，我会确认：集合为空时框架会不会生成 IN () 导致 SQL 报错或直接忽略条件。

我们后来在团队规范里加了一条铁律：UPDATE 和 DELETE 语句必须带 LIMIT 1 或明确的数量限制，除非是 DBA 审核过的批量任务。这个习惯帮我拦住了不止一次“误全表”的风险。

2. 外部调用：超时、降级、异常，一个都不能少

有次同事写了个调用第三方风控服务的代码，评审时我注意到他没有设超时时间，也没有降级逻辑。我问：“如果这个服务挂了，你的接口会怎样？”他说：“应该不会吧，对方 SLA 99.99%。”结果上线第二周，对方服务真的超时了，我们的线程池被拖满，整个下单链路雪崩。

从那以后，我评审外部调用时必查三样东西：超时配置、熔断降级、异常捕获后的兜底策略。不是不信任第三方，是生产环境的墨菲定律——会出错的终将出错。

3. 并发与边界：别让你的代码在“万一”时翻车

多线程、分布式锁、幂等性、边界值——这些地方最容易出隐蔽 Bug。评审时我会刻意想一些“变态场景”：如果同一毫秒来了两个相同请求会怎样？如果 Redis 锁刚好在业务执行完之前过期了会怎样？如果金额是 0.01 而不是 100 会怎样？

有个新人写了个“领取优惠券”的功能，逻辑看起来很完整。但我注意到他用的是 SELECT COUNT(*) FROM coupon WHERE user_id = ? 来判断是否已领，然后在应用层判断数量。我问他：“如果并发请求同时通过了这个检查呢？”他愣了一下，后来改成了数据库唯一索引 + 原子操作，避免了超领风险。

4. 日志与可观测性：出了问题能查吗？

我常问一个问题：“如果这段代码在生产报错了，你能在 5 分钟内定位原因吗？”如果日志里只有 ERROR: process failed，没有关键参数、没有上下文，我会打回去要求补充。好的日志不是越多越好，而是在关键决策点留下痕迹。

评审不是挑刺，是结对编程

很多人觉得评审就是找茬，其实不然。好的评审是建设性的对话。我会写这样的评论：

“这里用 HashMap 在并发场景下可能有问题，要不要考虑 ConcurrentHashMap？或者如果写多读少，用 synchronized 也行。”

“这个 SQL 在大数据量下可能慢，要不要加个覆盖索引？我试了 EXPLAIN，加 (user_id, status) 组合索引应该能走 ref。”

这种交流让双方都学到了东西。我们团队有个不成文的规定：评审意见要具体、要给出替代方案、要带着为什么。纯吐槽“这代码写得烂”是不被接受的。

现在我团队的评审文化已经变了。以前大家嫌慢、嫌烦，现在新人主动找老人评审，因为知道多一双眼睛就少一个坑。我们的生产事故率比去年同期降了 60%，而评审所花的时间，平均每千行代码不到两小时——换算下来，每次评审可能帮你省掉的是通宵回滚的代价。

如果你现在的团队还在“LGTM 文化”里打转，不妨从下一次评审开始，认真问自己：如果这段代码今晚在生产炸了，我会不会后悔没多看一眼？