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

久而久之，我也学到了一些有效审查代码的方法。现在我关注的重点与刚入行时已大不相同。

着眼大局

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

好的审查不仅看改动本身，还会看这些改动解决了什么问题、未来可能引发什么问题，以及改动如何融入系统的整体设计。

我喜欢关注那些没有被改动的代码行。它们往往能揭示真相。

例如，人们常常会忘记更新代码库中相关的部分或文档。这可能导致缺陷、困惑、破坏性变更或安全问题。

要全面细致，查看新代码的所有调用点。它们是否都已正确更新？测试是否仍在验证正确的内容？改动是否放在了正确的位置？

这些问题更多关乎系统设计，而非改动本身。不要忽视大局，因为如果接受了糟糕的改动，系统会变得脆弱。

代码不是孤立编写的。更有经验的开发者的职责是降低项目的运作阻力并做好风险管理。文档、测试和数据类型与代码本身同等重要。

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

命名就是一切

在审查代码时，我会花大量时间思考好的命名。

给事物命名很难，正因如此，把名字起好才至关重要。很多时候，这也是代码审查中最重要的部分。

这也是最主观的部分，因此也最繁琐，因为很难区分到底是吹毛求疵，还是重要的命名决策。

命名封装了概念，是代码中的“构建块”。糟糕的命名是一种代码异味，暗示着深层次的问题。它们会让认知负担增加一个甚至多个数量级。

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

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。检出分支、到处改改、故意弄坏点东西、试着理解其工作原理，都是我审查流程的一部分。

像UI变更或错误信息这类面向用户的改动，通常在实际运行代码并尝试将其弄坏时更容易被发现。

之后，我会还原改动，并在需要时将发现写在评论中。这种方法能带来更深入的理解。

坦诚说明你的时间安排

代码审查常常是开发流程中的瓶颈，因为它无法完全自动化：需要有人参与其中，查看代码并提供反馈。

但如果要等同事来审查你的代码，可能会让人感到沮丧。不要成为那个造成等待的人。

有时你会没时间审查代码，这没关系。如果无法在合理时间内完成审查，请告知作者。

我在这方面仍在改进，但我会尽量更主动地说明自己的时间安排并设定清晰的预期。

永不停止学习

代码审查是我最喜欢的学习新事物的方式。我会学到新技术、新的模式、新的库，但最重要的是，了解他人解决问题的方式。

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

不要吹毛求疵

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

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

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

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

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

看看下面两条代码审查评论。第一条没有帮助且带有否定意味。

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

你更愿意收到哪一条？

我知道这需要更多的时间和精力，但这是值得的！大多数情况下，作者会心存感激，并避免将来再犯同样的错误。随着时间推移，有帮助的审查会产生复利效应。

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

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

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

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

就你的审查风格征求反馈

时不时地，请作者对你的反馈本身给出反馈：

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

基本上，就是请他们来审查你的审查流程，呵。

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