我的代码审查清单

引言

说起代码审查，这事儿我折腾了少说也有五六年了。从刚开始入行时被同事review得怀疑人生，到后来自己独立负责项目、再到带新人做review，这一路走过来，最大的体会就是：代码审查绝对不只是找bug。它更像是一次代码质量的“体检”，也是团队技术交流的重要方式。

我这些年养成了一个习惯——每次给别人做review，或者自己提交代码前，都会对照一份清单检查一遍。这份清单不是从什么书上看来的，而是踩了无数坑之后慢慢总结出来的。今天就把它分享出来，希望对大家有点帮助。

一、先看整体，再看细节

我review代码的时候，第一件事绝对不看具体实现，而是先看整体结构。这一步太重要了，很多人拿到PR直接就去看某一行代码写得好不好，这完全是捡了芝麻丢了西瓜。

首先我会问自己几个问题：这个改动的影响范围有多大？有没有改动到核心模块？如果改的是核心逻辑，我会格外小心。代码的目录结构是否合理？新增的函数应该放在哪个文件？有没有把不相关的逻辑塞进了已有的文件里？

有时候我看到一些代码，单看每一行都没问题，但就是觉得别扭。仔细一看，原来是职责不单一——一个函数既在处理业务逻辑，又在写数据库，还在发消息通知。遇到这种情况，我通常会建议作者把代码拆一拆。

# 好的做法
def create_order(user_id, items):
order = build_order(user_id, items)
save_order(order)
notify_user(order)
    
不好的做法 —— 一个函数干了三件事
def create_order(user_id, items):
# 校验
# 组装数据
# 写库
# 发消息
# 更新缓存
# 记录日志
# ... 一大坨

看整体还有一点，就是代码风格统一。如果项目里用的是驼峰命名，你就别用下划线；如果用的是4个空格缩进，你就别用tab。这事儿看起来是小事，但一堆PR里混着不同风格，后期维护起来真的头疼。

二、可读性是第一条

我一直觉得，代码首先是给人看的，其次才是给机器跑的。你自己写的代码，两个月后你自己可能都看不懂，更别说别人了。

我检查可读性的时候，首先看命名。变量名、函数名、类名能不能做到“望文生义”？我见过太多d1、temp、data这样的命名了，真的很想把作者拉过来聊聊。命名最好的状态是：别人看到这个变量名，大概能猜到它是干什么的，不需要再去看上下文。

// 不好的命名
let d = new Date();
let temp = users.filter(u => u.s === 'active');

// 好的命名
let currentDate = new Date();
let activeUsers = users.filter(user => user.status === 'active');

其次看注释。这里有个误区，不是说注释越多越好。有些代码写得清晰明了，根本不需要注释，你硬要加一堆注释反而碍事。我更在意的是：为什么这么写，而不是做了什么。实现细节不需要注释，但业务决策、技术选型的理由最好写清楚。

还有一点容易被忽略——函数长度。我个人的经验是，一个函数最好控制在50行以内。如果一个函数超过了一屏，你review的时候就得仔细想想，是不是可以拆成几个小函数？函数越长，出bug的概率越高，测试越难写，别人越不愿意看。

三、逻辑和边界条件

这部分是技术含量最高的，也是最容易出bug的地方。

我review代码时，会特别注意边界条件。比如数组下标会不会越界？空指针有没有处理？除法运算分母是不是可能为零？循环有没有死循环的风险？这些看起来是常识，但实际开发中真的是一不小心就踩坑。

# 可能出问题的地方
def get_user_by_id(users, user_id):
return users[user_id]  # 如果user_id不存在呢？

改进一下
def get_user_by_id(users, user_id):
return users.get(user_id)  # 或者返回None，或者抛异常

还有一个容易漏掉的地方是并发问题。如果代码涉及到多线程、异步操作，一定要看看有没有竞态条件、脏读这些问题。我之前review过一个代码，表面上看逻辑没问题，结果上线后偶尔就出问题，后来定位半天发现是并发写入没有加锁。

四、安全和错误处理

安全这事儿怎么说都不为过。我review代码时，安全相关的问题是绝对不放过的。

首先看输入验证。所有来自外部的输入——用户表单、API参数、数据库查询结果——都必须校验。SQL注入、XSS、CSRF这些老生常谈的漏洞，原理大家都懂，但真的review起来，还是能发现不少问题。

# 危险的做法
query = f"SELECT * FROM users WHERE name = '{username}'"

安全的做法
query = "SELECT * FROM users WHERE name = %s"
cursor.execute(query, (username,))

其次看权限校验。这个接口需不需要登录？需不需要特定权限？有没有遗漏某些角色的访问控制？后端代码尤其要注意，前端做权限控制那是防君子不防小人，后端才是最后一道防线。

还有一点是敏感信息。密码、token、密钥这些绝对不能明文存储或传输。配置信息要放在环境变量里，不要硬编码在代码中。

五、测试和文档

最后一部分往往被忽略，但我觉得同样重要。

# 简单的测试用例示例
def test_calculate_discount():
assert calculate_discount(100, 0.2) == 80
assert calculate_discount(100, 0) == 100
assert calculate_discount(100, 1) == 0
# 边界情况
assert calculate_discount(0, 0.5) == 0

如果你的改动涉及API变更，接口文档更新了吗？参数说明、返回值格式、错误码含义，这些都要同步更新。我发现很多团队代码写得快，但文档永远跟不上，最后新来的同事只能靠猜。

还有一点是变更说明。每次提交PR，最好写清楚这个改动是为了解决什么问题，影响范围是什么，有没有需要注意的副作用。这不仅帮助reviewer理解，也方便以后追溯。

总结

以上就是我做代码审查时的 checklist，说白了就是五个方面：整体结构、可读性、逻辑边界、安全错误处理、测试文档。

这套清单我用了好几年，帮自己和团队避免了不少问题。当然了，也不是说每次review都要逐条照搬，灵活运用最重要。简单的改动可以快一点，复杂的逻辑就得仔细看。

最后想说，代码审查真的是个技术活，也是个耐心活。它不只是挑毛病，更是帮助团队提升代码质量、传播最佳实践的过程。有时候你提的一个建议，可能作者受益一辈子。

好了，这就是我的分享，希望能对你有点启发。如果你们团队有更好的review经验，欢迎留言交流～