
代码审查的残酷真相:为什么高级开发者从不纠结命名和格式?
看完这篇,你会发现自己过去的Code Review可能都是在浪费时间。
你花了两天时间精心实现一个功能,提交PR后满怀期待等待Review。结果收到20条评论:
你逐条修改,再次提交。然后代码上线了,生产环境炸了——因为并发场景下的race condition没人发现。
这就是大部分团队Code Review的现状:把时间花在不重要的细节上,真正致命的问题却视而不见。
作为一个在前端摸爬滚打多年的老兵,我见过太多这样的场景。今天我想聊聊,真正的高级开发者是怎么做Code Review的——以及为什么他们的方式和你想的完全不同。
打开一个PR,99%的人第一反应是看代码实现:
但高级开发者的第一个问题是:这段代码应该存在吗?
我见过一个经典案例。某同学提交了一个完美的虚拟滚动实现,处理了边界情况,写了完整测试,代码优雅得让人想点赞:
// PR标题: 为表格添加虚拟滚动优化性能
function VirtualizedTable({ data, rowHeight = 50 }) {
const [scrollTop, setScrollTop] = useState(0);
const containerHeight = 600;
// 计算可见区域
const visibleStart = Math.floor(scrollTop / rowHeight);
const visibleEnd = Math.ceil((scrollTop + containerHeight) / rowHeight);
const visibleData = data.slice(visibleStart, visibleEnd);
return (
<div
style={{ height: containerHeight, overflow: 'auto' }}
onScroll={e => setScrollTop(e.target.scrollTop)}
>
<div style={{ height: data.length * rowHeight }}>
<div style={{ transform: `translateY(${visibleStart * rowHeight}px)` }}>
{visibleData.map(row => <TableRow key={row.id} data={row} />)}
</div>
</div>
</div>
);
}
代码挑不出毛病。但问题在于:为什么要一次性加载10000条数据?后端API明明支持分页,为什么不用?
虚拟滚动解决的是"如何高效渲染大量DOM"的问题。但真正的问题是"为什么要在前端渲染这么多数据"。这就像你家着火了,你选择买个更好的灭火器,而不是找出起火原因。
真正的Code Review应该质疑解决方案本身,而不是解决方案的实现质量。
这需要跳出代码看问题:
这种思维方式,才是高级开发者和普通开发者的分水岭。
我有个controversial的观点:大部分代码细节根本不重要。
userData还是user? 无所谓if-else还是三元运算符? 无所谓.map()还是for循环? 也无所谓看到这里可能有人要跳起来:"代码质量很重要!细节决定成败!"
我没说代码质量不重要。我是说:注意力是稀缺资源,应该花在刀刃上。
这里有个建筑学的概念很有启发——承重墙和非承重墙。承重墙支撑整个结构,拆了房子就塌;非承重墙只是分隔空间,改了无伤大雅。
代码也一样。有些决策是"承重"的:
这些值得死磕。但更多的是"非承重"的偏好:
// 这三种写法值得你留10条评论吗?
// 写法1
const isValid = data.name && data.email;
// 写法2
const isValid = Boolean(data.name && data.email);
// 写法3
const hasName = !!data.name;
const hasEmail = !!data.email;
const isValid = hasName && hasEmail;
写法3可读性可能好一点点。但值得让作者改代码、推新commit、延迟功能上线吗?
高级开发者知道什么时候该闭嘴。 不是因为妥协代码质量,而是因为他们清楚:在无关紧要的问题上消耗团队精力,就没时间关注真正重要的事了。
经历过几次生产事故后,你就会形成一种直觉——知道哪些代码模式容易出问题。
最容易被忽视的就是错误处理。 不是"有没有try-catch",而是"出错了会怎样"?
async function getUserProfile(userId) {
const response = await fetch(`/api/users/${userId}`);
const data = await response.json();
return data;
}
这代码开发环境跑得好好的。但上线后:
初级Review可能会说:"加个try-catch吧"。
高级Review会问:"用户体验应该是什么?显示错误提示?重试?用缓存数据?哪种失败模式对用户最友好?"
这才是关键差异——不是检查代码在理想情况下能不能跑,而是思考在最糟糕的情况下用户会遇到什么。
所有系统都会失败。API会挂,网络会断,数据库会超时,第三方服务会返回垃圾数据,用户会做你意想不到的操作。真正的代码审查,是在代码进入生产前,就预见到这些失败场景。
优秀的Code Review不只看写了什么,更要问:没写什么?
function useUserData(userId) {
const [data, setData] = useState(null);
useEffect(() => {
fetchUser(userId).then(setData);
}, [userId]);
return data;
}
这代码看起来没问题。但高级开发者会问:
"如果userId变化了,但上一个请求还没返回呢?组件会先设置旧数据,再被新数据覆盖吗?"
这就是经典的race condition。用户快速切换tab,页面显示错乱,没人知道为什么。
修复其实很简单,但前提是你要想到去找这个问题:
function useUserData(userId) {
const [data, setData] = useState(null);
useEffect(() => {
let cancelled = false;
fetchUser(userId).then(user => {
if (!cancelled) setData(user);
});
return() => { cancelled = true };
}, [userId]);
return data;
}
这类问题需要你在脑子里"运行"代码:
线性阅读代码是不够的,你需要在多个维度上模拟执行。 这是经验积累的结果,也是高级开发者最值钱的技能之一。
这可能是Code Review里最微妙的技能:判断什么问题必须现在解决,什么可以后续优化。
必须Block的:
可以放行的:
对比这两段代码:
// 这个必须Block - 会留下一堆脏数据
asyncfunction deleteUser(userId) {
await db.users.delete(userId);
// 等等,用户的帖子、评论、关系怎么办?
// 删不干净会导致数据一致性问题
}
// 这个可以放行 - 只是风格问题
function getUserDisplayName(user) {
return user.firstName + ' ' + user.lastName;
// 可以用模板字符串,但这样也能用
}
第一个会在生产环境造成数据混乱,第二个只是写法偏好。
判断标准很简单:这会导致生产事故吗?会让代码库显著难维护吗? 如果答案是否,建议改进但别阻止合并。
对比这两条Review评论:
弱评论: "这里应该用useMemo"
强评论: "这个计算每次渲染都会跑,处理1000+项目。用useMemo包裹,依赖项设为[items],可以避免其他状态变化时重复计算。我本地profiling显示渲染时间从80ms降到5ms。"
差异在哪?
第二条评论不只说了做什么,还解释了:
当你解释"为什么"时,你不是在给指令,而是在传授思维方式。 开发者学到的不是规则,而是原则。下次遇到类似场景,他们就能自己判断该怎么做。
这在反对某种方案时尤其重要:
❌ "这个方法不对" ✅ "这个方法在下个sprint加入多用户功能时会出问题,因为它假设只有一个活跃用户。我们需要重构成支持多用户的结构,现在改比上线后改容易得多。"
第二种方式提供了上下文,让开发者理解更大的图景。这样的反馈引导理解,而不只是强制服从。
代码的生命周期远比你想象的长。今天你花2小时写的功能,可能半年后需要改动。那时候:
高级开发者审查代码时,想的是:"这段代码能不能脱离作者独立存在?"
// 缺少上下文 - 半年后没人知道0.88是什么
function calculatePrice(item) {
return item.basePrice * 0.88;
}
// 有清晰上下文 - 未来维护者能理解
function calculatePrice(item) {
// 扣除12%平台手续费 (0.88 = 1 - 0.12)
// 详见定价文档: https://docs.company.com/pricing
return item.basePrice * 0.88;
}
魔法数字0.88让人一头雾水。注释解释了它的含义和出处,未来的开发者不用到处找文档或问人。
同样重要的是识别"技术上正确但让人困惑"的代码:
// 能跑,但看着累
const isValid = !!data && !!data.name && data.name.length > 0;
// 同样逻辑,意图清晰
const hasValidName = data?.name && data.name.length > 0;
第二种写法更容易理解。有人快速浏览代码时,不用解析布尔逻辑就能明白在检查什么。
可维护性不是奢侈品,而是必需品。 每次Review都是在为未来投资,减少技术债务,让代码库保持健康。
当一个PR方向根本就错了,你写再多评论也没用:
这时候别写长篇大论的评论,直接拉个会聊。
"嘿,我觉得咱们应该聊一下这个PR。我对整体方案有些担忧,实时讨论会比来回留言快。"
当面讨论的好处:
Code Review评论适合处理具体的、局部的修改建议。对于需要重新思考整体方案的情况,它就是错误的沟通方式。
知道什么时候切换沟通渠道,这是高级开发者的关键软技能。
做Code Review最难的部分不是技术判断,而是社交平衡。
你需要:
这种平衡很难把握。太严格会打击积极性,太宽松会放过问题。
一个技巧是从好奇开始,而不是纠正:
❌ "你应该用方案Y而不是X" ✅ "我好奇为什么选择方案X而不是Y? 我之前遇到过Y在这种场景下更合适的情况"
第二种说法:
同时,该教的时候别藏着掖着。初级开发者不知道自己不知道什么。你发现的模式和坑,如果不解释清楚,他们下次还会犯。
这种社交技能——在信任、教导和共情之间找平衡——才是区分高级审查者和一般审查者的关键。
Code Review不是追求完美,而是在保证质量的前提下让代码尽快上线。
每条评论都有成本:
所以问题不是"什么地方可以更好?"——因为代码永远可以更好。
真正的问题是:"什么必须在上线前改变?"
这个列表要短得多:
其他的:
最好的Code Review是那些发现了微妙的race condition或架构问题,避免了几周的痛苦的Review。 不是那些纠结变量命名和格式的Review。
知道区别。审查真正重要的东西。
如果你读到这里,我希望你能重新思考Code Review的目的。
它不是展示你知道多少规则的舞台,不是刷存在感的机会,更不是追求完美代码的工具。
Code Review的真正价值在于:在代码进入生产前,用经验和判断力筛选出真正重要的问题。
下次做Review时,问问自己:
如果答案是否,就让它过。把精力留给真正重要的战斗。
因为注意力是有限的,leverage才是无限的。
你觉得呢?欢迎在评论区分享你遇到过的"最没意义"和"最有价值"的Code Review经历。