来源： piglei
Code Review（代码评审）是一种流行的软件开发实践。通过在代码合入主分支前引入人工评审，能有效促进成员间的知识交流，提升软件质量。
我以评审者的身份参与过大量代码评审。在评审一份代码时，有些事项长期处在我的关注榜头部，比如设计是否考虑到了边界情况、代码是否有合理的单测覆盖。也有一些事项，因看似无关痛痒一直未引起足够重视，直到最近，我才渐渐发现它们的重要性。
以下是曾被我忽视的 3 件重要的小事。
1. 命名
小女孩千寻误入汤婆婆为神明开设的浴场。为了留在浴场内工作，千寻与汤婆婆签订了一份协议，但协议并非重点，重点是另一件看似无关紧要的小事——汤婆婆给千寻改了个名：从「千寻」改为「千」。一旦失去了原本的名字，人们便失去了逃离浴场所在的异世界的能力，甘心永世被汤婆婆所奴役。
——电影【千与千寻】
程序员们对「命名」的关注程度似乎呈一个「倒 U 形」曲线。缺乏经验时，对命名的关注度很低，代码中充斥着各类不准确、不精确的名字，无法有效描述各种抽象概念。
下面这段代码中的命名就存在不少问题：
defget_var(u):"""获取环境变量列表""" data1 = UserVarsManager.get(u) data2 = SiteVarsManager.get(u.site)return data1 + data2
随着经验逐渐增加，大家对命名的关注度逐步提升。项目中的名字开始变得更具有描述性，含糊不清的名字渐渐绝迹。名字至少不会成为他人理解代码时的屏障。
这个阶段，代码会逐渐演变成像是这样：
deflist_environment_vars(user):"""获取环境变量列表""" items_user = UserVarsManager.get(user) items_site = SiteVarsManager.get(user.site)return items_user + items_site
在绝大多数评审中，这绝对算是一份合格的代码，至少不大可能因为命名应发争议。
自此之后，大部分程序员们对命名的关注度进入「倒 U 形」曲线的后半段： 不再如从前那般关注命名，名字只要有一定描述性，不造成歧义就足够。 我也曾经是其中一员。
但不应在这个阶段停留太久，作为代码评审人，我们应该不断提升自己对于名字的敏感度。比方说，对于前面那份代码，也许应该提出以下评审建议：
deflist_environment_vars(user):# 1"""获取环境变量列表""" items_user = UserVarsManager.get(user) # 2 items_site = SiteVarsManager.get(user.site)return items_user + items_site
•
评论 1：
项目中对于「环境变量」的统一缩写是
env_variables /env_vars
，此处应保持一致，使用
list_env_variables
或
list_env_vars
。
•
评论 2：
UserVarsManager.get
的命名可优化，因为
Manager
是一个「万金油」名词，虽然放在各种场景下都不违和，但也是以损失名字（等同于「职责」）的精确指向性为代价，此处可考虑改用一个更精确的名字，比如：
UserEnvVarsRetriever.get(user)
；
SiteVarsManager
同理。
虽然只是两处小改进，但是积少成多。
每一次代码评审，必定涉及到许多新名字。但名字并非生来平等，不是所有名字都值得我们花费时间，应当尽量把关注点聚焦在那些最常被使用、最靠近用户的名字上，比如 URL 路径的资源名、数据库模型与字段名、工具函数（类方法）名，等等。
此外，与业务直接相关的领域词汇重要程度极高。评审时，每一个关键的领域词汇都值得仔细斟酌、反复推敲。举个例子，开发一个影评功能，」用户评分「、「媒体评分」、「平均分」分别该用哪些名字表示？你绝不会想要在一个文件里看到
movie_score
，在另一个文件里看到
movie_rating
。
命名这件小事，虽然看似不起眼，但项目规模越大、所跨越的时间维度越长，在名字质量上的细微差别就越容易累加出不可估量的巨大影响。
2. 指引性注释
夏洛已经在网上织出了光彩照人四个大字，威尔伯站在金色的阳光里，真是光彩照人。自从蜘蛛开始扶助它，它就尽力活得跟它的名声相衬。夏洛的网说它是王牌猪，威尔伯尽力让自己看上去是只王牌猪；夏洛的网说它了不起，威尔伯尽力让自己看上去了不起；现在网上说它光彩照人，它尽力让自己光彩照人。
——【夏洛的网】
关于注释，我向来信奉 Bob 大叔在 【代码整洁之道】 [1] 里的观点： 「注释的恰当用法是弥补我们在用代码表达意图时遭遇的失败。」 这就是说，好代码应该总是能清晰说明自身意图，无需注释再来画蛇添足，注释只应该被用来描述那些代码之外的信息，比如解释「为什么」。
正因如此，注释总是应该被谨慎使用。假如一段代码很难理解，第一反应不应该是补注释，而是应该去追求用一种更易理解的方式重写它。
但随着时间的推移，我渐渐意识到，事情不能一概而论。「指引性注释」，或者说常被人们诟病为「近乎复述代码意图」的描述性文字，也有着不可替代的重要作用。
Redis 的作者 antirez 就是「指引性注释」的忠实拥护者，他曾写过 一篇文章 [2] 详细分析过指引性注释在 Redis 项目中的应用。下面这段代码摘自 Redis 源码，里面就有不少「指引性注释」：
/* Call the node callback if any, and replace the node pointer * if the callback returns true. */if (it->node_cb && it->node_cb(&it->node))memcpy(cp,&it->node,sizeof(it->node));/* For "next" step, stop every time we find a key along the * way, since the key is lexicographically smaller compared to * what follows in the sub-children. */if (it->node->iskey) { it->data = raxGetData(it->node);return1; }
在这段代码中，两段注释并未提供任何在代码之外的新信息。所以，好处是什么？
最直观的好处，就是这些注释让代码变得更容易理解了，它们极大地降低了人们阅读代码时所需付出的心智成本。同样一份代码，在缺少指引性注释的情况下，完全理解它的行为可能得花费 10 分钟，而有了注释的帮助，时间也许就能缩短到 5 分钟甚至更短。
当新开发者加入项目时，这些指引性注释也能助力他们更快上手。
正因如此，在评审一份代码时，我常常会在一段复杂的代码逻辑上评论： 「Nit：考虑增加一小段指引性注释，帮助理清代码行为。」（Nit=nitpick，表示「鸡蛋里挑骨头」式的并不强烈要求修改的意见） 。
此外，如果一段代码曾在评审过程中引发过一些深度讨论，那么那些讨论内容，也许很适合被二次加工后，作为指引性注释加入代码中。对于理解代码来说，它们有时有奇效。
不过，在追求「指引性注释」的路上，也要避免踩入以下几个陷阱：
• 简单复述代码 ：指引性注释虽然是一种帮助理解代码的辅助性文字，但绝不能只是复述代码而已，简单来说，你可以这么理解两者在传递信息方面的风格差异：代码是一本厚厚的权威科学教材，指引性注释则是一小册面向中学生的科学启蒙读物 • 追求「注释率」 ：不要在「代码注释率」指标上设置硬性要求，指引性文字也需要讲究质量，盲目追求数量只会适得其反 • 不注重时效性 ：过时的注释比代码危害更大，要及时修改或删除已经过期的指引性注释
总而言之，你可以把指引性注释当成有针对性的代码「教学文本」。审阅代码时，如果你发现一段逻辑理解起来很吃力，而代码本身也没有太多优化空间，请不要迟疑，勇敢表达出你对于「教学文本」的需求吧！
3. 沟通方式
「我因为鲁思和萨拉不得不离开我们而痛苦万分。而令我感觉更加痛苦的是我当时以为自己是完全孤立无援的。」
「说真的，肯顿小姐……」我端起那个我用来放使用过的瓷器的托盘。「对那样的解雇我自然是极不赞同的。我还以为那是不言自明的。」
——【长日将尽】
时至今日，仍有许多人认为软件开发是一种单打独斗的工作。一位程序员捡起一块键盘，就能源源不断地产出代码，根本不需要其他人。但事实是，程序员单打独斗的黄金时代早已过去，现代软件开发已演变成一种多人参与的协作事务。正因如此，程序员的日常充斥着各类沟通工作，参与代码评审正是其中之一。
在代码评审时，评审者的工作内容似乎一句话就可简单概括： 指出他人代码中的不足。 这听起来易如反掌，对不对？我曾经正是这么以为，所谓评审，只要做完下面的「123」即可：
1. 找出所有可优化的点（有事说事！） 2. 等提交者完成改动，或在讨论后确认维持原状（就事论事！） 3. 合并代码（大功告成！）
而现实总是和理想相去甚远，代码评审很少会像上面这样顺利。因为一旦涉及到人与人之间的沟通，尤其其中一方还在给另外一方「挑毛病」，事情又能简单到哪儿去呢？
人类是一种神奇的智慧生物，阅读一段文字，不仅能从中获取到信息，更能从字里行间感受到 情绪 ，有时，这份情绪甚至会盖过信息，影响他们做出判断。因此，当你在参与评审时，请谨记这一点：保持谦逊、尊重他人，无论对方的经验或背景如何。优秀的表达，能做到内容即使在批评，也能让对方感受到自己仍是被尊重的。
让我来举一个例子。团队内来了一位新人，用他不太熟悉的 Python 语言提交了一个 PR。作为 PR 的评审人，你在代码里发现了一段冗长的循环代码，于是写下评论：
代码比较啰嗦，建议改成列表推导式。
虽然你的观点没错，但这种表达方式值得商榷。下面是这条评论的另一种写法：
这里的循环体较简单，只有过滤和转换逻辑，很适合改成列表推导式，代码更精简。举个例子：items = [to_item(obj) for obj in objs if obj.is_valid()]参考：https://docs.python.org/3/tutorial/datastructures.html#list-comprehensions
比起第一条评论，后面这条显然更不容易引起新人的抵触情绪，更可能被采纳。这就像那句老话所说： 「你表达的方式跟所要表达的内容同样重要，如果不是更重要的话。」
除了保持谦逊和尊重，还有一些其他值得采用的评审沟通小技巧：
• 一例胜千言 ：有时用文字洋洋洒洒写一大堆，不如直接写几行代码，举一个实际的代码样例 • 见什么人说什么话 ：对于加入团队一个月和一年的开发者，你在评审时可以（也应该）区别对待；对缺乏经验的新人，组织语言时要谨慎，尽量避免让对方感觉不被尊重，产生太多的挫败感；对熟稔的老人，语言风格就可以相对随意，言简意赅即可，不必过于啰嗦
我相信也许大部分人在心底都认同：代码评审是一个「对事不对人」的过程，不应该把对代码的批评当成对人的否定。但这和提倡「好好说话」并不冲突。一次让双方满意的沟通，几乎等同于一次更高效的沟通。所以，改善沟通方式就能提升工作效率，你又何乐而不为呢？
总结
代码评审作为保障软件质量的重要手段，是大型软件开发中不可或缺的重要一环。本文总结了我作为评审的参与者，在命名、指引性注释和沟通方式三个方面的一些思考，要点如下：
• 程序员们应当不断提高在代码评审时对于命名的敏感度 • 检查命名的两个技巧：同类名词保持一致、用更精确的词代替那些「万金油」名字 • 对待名字不要一视同仁，多多关注那些最 重要 的名字 • 对任何一个项目，领域（业务）相关的名字最重要，值得仔细斟酌、反复推敲 • 指引性注释虽不提供太多代码之外的信息，但也有着不可替代的作用 • 指引性注释能大大降低人们为理解代码所付出的心智成本 • 留意指引性注释的几个陷阱：简单复述代码、追求「注释率」、不注重时效性 • 评审时，要勇于对复杂的代码逻辑提出补充指引性注释的请求 • 软件开发中包含许多与沟通有关的事项，代码评审正是其中之一 • 理想的评审是「有事说事，就事论事」的，但正因涉及人际沟通，导致现实往往偏离理想 • 文字不光能传达信息，更是情绪的载体，而情绪往往会影响沟通的效果 • 在评审中，永远保持谦逊、尊重他人，无论对方的经验或背景如何 • 对于有着不同经验的待评审者，应当采取不同的沟通风格
代码评审是一项涉及多人协作的复杂事务，里面藏着许许多多的学问。质量高的评审，对于提升质量和塑造团队氛围有着不可替代的作用。质量低下的评审，则可能沦落为形式主义，甚至让团队内部滋生矛盾和不满。
而影响评审质量的因素，往往藏在那些不起眼的小细节、小事情中。以上这些关于「小事情」的经验之谈，希望能对你的工作有所启发。
题图来源: Photo by Helena Lopes on Unsplash
References
[1]
【代码整洁之道】:
https://book.douban.com/subject/4199741/
[2]
一篇文章:
http://antirez.com/news/124
以上是今天的分享，最后推荐一下我的【Python潮流周刊】专栏。
这是一个专为国内 Python 开发者量身打造的资讯平台，为你挑选最值得分享的文章、教程、开源项目、软件工具、播客和视频、热门话题等内容。
如果你觉得本文有帮助
请慷慨 分享 和 点赞 ，感谢啦 ！
