我评审他人代码已经有一段时间了，准确地说，已经超过二十年。如今，我有 50% 到 70% 的时间都在以各种形式评审代码。这就是我的本职工作，此外还有系统设计。

久而久之，我也总结出了一些关于如何高效评审代码的心得。现在我关注的重点，已经和刚入行时大不相同。

着眼大局

糟糕的评审往往视野狭窄，只盯着语法、风格和细枝末节，却忽略了可维护性和可扩展性。

好的评审不仅会看改了什么，还会看这些改动解决了什么问题、未来可能带来什么隐患，以及它如何融入系统的整体设计。

我喜欢去看那些没有被改动的代码行。它们往往才道出了真相。

例如，人们常常会忘记更新代码库或文档中相关的部分，这就可能导致 Bug、混乱、不兼容的变更，甚至安全问题。

要足够细致，查看新代码的所有调用点。它们都已正确更新了吗？测试还在验证正确的内容吗？这些改动放在正确的位置了吗？

这些问题与其说是关于改动本身，不如说是关于系统设计。不要忽视大局，因为一旦接受了糟糕的改动，系统就会变得脆弱。

代码不是孤立编写的。更有经验的开发者的职责是为项目减少协作摩擦、做好风险管理。文档、测试和数据类型与代码本身同等重要。

随着代码的演进，要始终留意更优的抽象方式。

命名就是一切

评审代码时，我会花很大一部分时间去琢磨命名是否得当。

给事物命名很难，正因如此，才更要把它做好。很多时候，这甚至是代码评审中最重要的一环。

它也是最主观的部分，这让它变得很棘手，因为很难分清到底是在吹毛求疵，还是在做至关重要的命名决策。

命名封装了概念，是代码中的“积木”。糟糕的命名是一种代码异味，暗示着更深层次的问题。它会让认知负担成倍甚至成数量级地增加。

例如，假设我们有一个表示游戏中玩家数据的结构体：

struct Player {
    username: String,
    score: i32,
    level: i32,
}

我经常看到这样的代码：

// Bad: using temporary/arbitrary names creates confusion
fn update_player_stats(player: Player, bonus_points: i32, level_up: bool) -> Player {
    let usr = player.username.trim().to_lowercase();
    let updated_score = player.score + bonus_points;
    let l = if level_up { player.level + 1 } else { player.level };
    let l2 = if l > 100 { 100 } else { l };

    Player {
        username: usr,
        score: updated_score, 
        level: l2,
    }
}

这段代码很难读懂。 usr、updated_score 和 l2 到底是什么？意图没有被清晰地传达出来。这会堆积认知负担，让人更难理清逻辑。

所以我总会为变量斟酌最贴切的名字，哪怕显得有些吹毛求疵。

// Good: meaningful names that describe the transformation at each step
fn update_player_stats(player: Player, bonus_points: i32, level_up: bool) -> Player {
    // Each variable name describes what the value represents
    let username = player.username.trim().to_lowercase();
    let score = player.score + bonus_points;

    // Use shadowed variables to clarify intent
    let level = if level_up { player.level + 1 } else { player.level };
    let level = if level > 100 { 100 } else { level };

    // If done correctly, the final variable names
    // often match the struct's field names
    Player {
        username,
        score,
        level,
    }
}

在更大的代码库中，好的命名尤为关键——在那里，值的声明处与使用处相隔甚远，许多开发者也必须对问题域达成共识。

不要害怕说“不”

我经常不得不拒绝一些改动，这从来都不容易。毕竟别人付出了很多努力，自然希望自己的成果能被接受。

不要粉饰你的决定，也不要为了显得和气而拐弯抹角。要保持客观，说明理由，并提供更好的替代方案。不要在拒绝上纠结，而是把重点放在下一步该怎么做。

与其接受一个不正确、日后会引发问题的改动，不如直接说不。一旦开了先例，以后再想拒绝类似的改动只会更难。

这正是评审流程存在的意义：代码并不保证一定会被接受。

在开源项目中，很多人贡献的代码并达不到标准。必须有人站出来说“不”，而这是个非常不讨好的工作（问问任何一位开源维护者就知道了）。然而，优秀的项目需要把关人，否则结果就是低劣的代码和最终无法维护的项目。

有时人们会说“先合进去，以后再修”。我认为这是滑坡的开始。这会导致技术债和日后更多的工作。坚持立场很难，但很重要。如果你发现不对劲的地方，就要说出来。

当你觉得难以开口时，请记住，你拒绝的是代码，而不是人。要让对方知道你感谢他们的付出，并且是想帮助他们做得更好。

即便你在评审中已经形成了该关注什么的直觉，也依然要用事实来支撑你的判断。如果你发现自己总是在为同一类问题说“不”，不妨为团队编写一份风格指南或规范。

要宽容，但也要果断；终究只是代码而已。

代码评审即沟通

代码评审不仅仅是关于代码，人也很重要。与同事建立良好的关系至关重要。

如果可能，我会特意把最初的几次评审放在结对编程中一起完成。

这样，你们可以了解彼此的沟通方式。建立信任、增进了解，用这种方式效果很好。如果之后发现沟通出现障碍或误解，也应该再次采用这种方式。

采用多轮评审

“能帮我快速看一下这个 PR 吗？我想今天就合进去。”人们常常以为代码评审是一次性的事。事实并非如此。相反，代码评审是一个迭代的过程。要把代码做到位，就应该预期会有多轮评审。

在第一轮评审中，我会关注大局和整体设计。完成这一步后，我再深入细节。

目标不应该是尽快合入，而应该是接受高质量的代码。否则，代码评审本身还有什么意义？这种思维方式的转变很重要。

评审并不仅仅是为了指出缺陷，也是为了在团队内对代码形成共识。我常常通过评审他人的代码学到最多关于如何写好代码的经验。我自己的代码也曾得到过优秀工程师非常宝贵的反馈。

这些都是无价的“顿悟时刻”，能帮助你作为开发者不断成长。专家们花了宝贵的时间来评审我的代码，我从中受益匪浅。我认为每个人在职业生涯中都应该经历一次这样的体验。

别做讨人厌的人

你时不时会与作者意见不一致。保持尊重、具有建设性很重要。要避免人身攻击或居高临下的语气。不要说“这是错的”，而要说“我会这样做”。如果对方犹豫不决，不妨多问几个问题，了解他们的思路。

• “如果我们这样做，会破坏现有工作流吗？”
• “你考虑过哪些替代方案？”
• “如果用空数组调用这个函数，会发生什么？”
• “如果我不设置这个值，呈现给用户的错误信息会是什么？”

这些“苏格拉底式提问”¹能帮助作者思考自己的决策，并可能带来更好的设计。

人们应该乐于收到你的反馈。如果不是这样，就该反思一下自己的评审风格。只写那些你自己也乐意收到的评论。

我时不时也会加上一些积极的评价，比如“我喜欢这里”或“这是个好主意”。让作者保持积极性，让他们感受到你对他们工作的认可，往往能起到很大的作用。

如果可能，试着运行一下代码

长时间盯着代码看，很容易忽略细微的细节。手头有一份可以在本地把玩的代码，对我帮助很大。

如果可以，我会尝试运行代码、测试和 linter。检出分支、随意改动、试着弄坏它、努力理解它的工作原理，这些都是我评审流程的一部分。

像界面改动或错误提示这类面向用户的变更，往往在实际运行并尝试破坏它时更容易被发现。

之后，我会还原这些改动，并在需要时把发现写进评论里。这样做能带来更深入的理解。

坦诚说明你的时间安排

代码评审常常是开发流程中的瓶颈，因为它无法完全自动化：必须有人去看代码并给出反馈。

但如果别人一直在等你评审代码，就会感到沮丧。别成为那样的人。

有时你确实没时间评审代码，这没关系。如果无法在合理的时间内完成评审，就告诉作者一声。

这一点我自己也还在努力，但我会尽量主动说明自己的时间安排，并设定清晰的预期。

永不停止学习

代码评审是我最喜欢的学习新事物的方式。我能学到新技术、新模式、新库，但最重要的是，能看到别人是如何处理问题的。

我会在每次评审中尽量学到一件新东西。只要能帮助整个团队提升和成长，这就不是浪费时间。

不要吹毛求疵

格式化工具的存在是有原因的：把空格和格式问题交给工具去处理。把精力留给真正重要的问题。

关注逻辑、设计、可维护性和正确性。避免那些不影响代码质量的主观偏好。

问问自己：这会影响功能吗？会让未来的开发者感到困惑吗？如果都不会，就放过它。

关注“为什么”，而非“怎么做”

评审代码时，要关注改动背后的原因。这比毫无理由地指出缺陷要有效得多。

看看下面两条代码评审评论。第一条毫无帮助，还带着否定意味。

第二条则提出了一个替代方案，附上了文档链接，并解释了这个改动日后可能导致的问题。

你更愿意收到哪一条？

我知道这样做需要更多时间和精力，但这是值得的！大多数情况下，作者会感激你的用心，并且以后会避免再犯同样的错误。日积月累，有帮助的评审会产生复利效应。

不要害怕问“愚蠢”的问题

提问总比臆测要好。如果你有不理解的地方，就请作者解释一下。很可能不止你一个人没看懂。

通常，作者会很乐意解释他们的思路。这样做能让你更好地理解代码和整个系统，也能帮助作者从不同的角度看问题。也许他们会发现自己的假设是错的，或者系统本身就不够直观。也许是缺少了文档？

会提好问题是一种超能力。

就你的评审风格征求反馈

时不时向作者征求一下对你反馈的反馈：

• 你是不是太苛刻/吹毛求疵/太慢/太马虎了？
• 你有没有指出正确的问题？
• 你的反馈对他们有帮助吗？
• 他们有没有改进的建议？

基本上，就是请他们来评审一下你的评审流程，哈哈。

学会如何评审代码是一项需要不断练习和打磨的技能。祝你早日找到属于自己的风格。