代码评审赋魅

image

先来看一个令无数技术Leader闻风丧胆的项目“死亡”三角,业务压力引发代码质量下降,代码质量下降引发开发效率下降,开发效率下降又加重了业务压力,最终导致业务压力山大,乃至项目烂尾。如何破解?方法有很多,像精简业务需求、增加开发人手、升级技术架构等,很多时候需要多管齐下,但凡打掉这个“死亡”三角中的任何一角,就能终止这个恶性循环,甚至逆转为良性循环。

代码评审(Code Revew,简称CR)的首要打击目标显然是“烂代码”。避免“烂代码”的最好时机是写代码的时候,其次是代码评审的时候。IBM 的 Orbit 项目有 50 万行代码,使用了 11 级的代码检查(其中包含代码评审),结果是项目提前交付,并且只有通常预期错误的 1%。一份对 AT&T 的一个 200 多人组织的研究报告显示,在引入代码评审后,生产率提高了 14%,缺陷减少了 90%。那到底什么是代码评审?如何进行代码评审?继续往下看。

1 CR 祛魅

我个人认为代码有这几种级别:1)可编译,2)可运行,3)可测试,4)可读,5)可维护,6)可重用。通过自动化测试的代码只能达到第3)级,而通过CODE REVIEW的代码少会在第4)级甚至更高。

—— 陈皓

下面 8 条有关 CR 的阐述,你觉得哪些是正确的?

  1. 搞形式主义,存粹是浪费时间
  2. CR 是保证程序正确性的手段
  3. CR 是保证代码规范的手段
  4. CR 是 Leader 的事,跟我没关系
  5. 我只看指给我的 CR,其他 CR 跟我没关系
  6. 没有时间 CR,直接 Merge
  7. CR 必须一行不落从头看到尾
  8. CR 必须一次完成

———————————————————————————————— 请仔细思考 60 秒

3...2...1...时间到,你的答案是几条?很抱歉,在我看来,没有一条是正确的。1、4、5、6 是送分题,显然都是错误的。7 是眉毛胡子一把抓,CR 就像读书,不是所有的书都适合精度,也不是所有的代码都需要评审。8 是任务心态,为了 CR 而 CR,CR 的目的不是完成 CR,而在于提升代码质量,你写代码时也不会一次完成所有功能。比较有争议的是 2 和 3,诚然,正确性和代码规范都是 CR 要关注的方面,但这并不意味着 CR 要保证正确性和代码规范(CR 也没法保证),保证正确性的主要手段是测试(单元测试,集成测试,契约测试,功能测试,自动化测试等),而保证代码规范主要依靠代码规范检查工具(像常用的 checkstyle 和 PMD)。

image

CR 到底是什么?依我所见,CR 本质上是一种讨论,一种严肃的、专业的、异步的以文字形式呈现的讨论,随意性和情绪化是 CR 的大忌。什么叫随意性?未经审视的评论。什么叫情绪化?因时而异,因人而异。高水平的 CR 首先要忘掉自己。

image

2 知:CR 的三重境界

技术水平决定了 CR 的下限,认知高度决定了 CR 的上限。所以说 CR 水平高不高,最终还是看认知水平。认识 CR 有三重境界,分别是执行层、团队层和文化层。

2.1 执行层:昨夜西风凋碧树,独上高楼,望尽天涯路

第一层为执行层,顾名思义就是通过如何做来认识 CR。以下列举 CR 时需重点关注的六个方面,并辅以相应的例子便于理解。

1)关注代码规范。命名是第一位的,一个令人费解的命名背后往往隐藏着一个设计纰漏。其他诸如空白字符、换行、注释等问题,也会影响代码的可读性和可理解性。

image
image

2)避免重复代码。编程法则第一条,Don't repeat yourself. 重复代码是万恶之首,重复代码人人得而诛之!

image

3)降低圈复杂度。什么是圈复杂度?简单来说就是代码中 if/case/for/while 出现的次数。圈复杂度越高,BUG 率越高。如果一个方法的圈复杂度达到 3 或者更高,那么 CR 时就要多看两眼。

4)关注性能问题。性能问题虽然不常见,可一旦暴雷往往就是大问题。CR 时看到循环,记得多留一个心眼。

image

5)关注分布式事务。涉及远程服务调用,或者跨库更新的场景,都应考虑是否存在分布式事务问题,以及适用何种处理方式,是依赖框架保证强一致性,还是记录异常数据保证最终一致性,抑或是直接忽略?

6)关注架构设计。代码有代码规范,架构有架构规范。面对一个新功能的 MR(Merge Request),除了检查架构规范,还应推敲其架构设计,比如是否符合整洁架构三原则,无依赖环原则,稳定依赖原则,稳定抽象原则。

image
image

除了线上 CR,还有一种特殊的线下 CR 方法,就是跳过 MR,直接拉取代码,进行整体 CR,将评审意见在代码中标记为 TODO 或者 FIXME,然后 @ 相关开发进行改进。这样做最大的好处,就是避免受单个 MR 的影响,掉入只见树木不见森林的陷阱。

image

2.2 团队层:衣带渐宽终不悔,为伊消得人憔悴

接下来再看第二层,如何从团队视角认识 CR。前面说了,CR 本质上是一种讨论,培根说过「读书使人完整,讨论使人完备」,从个人到团队,CR 分别意味着什么?

  • 提升自我觉察能力:这是从个人视角来看,当你知道你写的代码会有另一双眼睛来审阅,那你写代码时就会保持一份警觉,放弃天知、地知、我知、你不知的幻想,认认真真写好每一行代码。

  • 建立良好开发节奏:这是从团队视角来看,CR 是团队的同步器,每个人既是自己 MR 的作者,又是别人 MR 的评审者,从 MR 到 CR,再从 CR 到 MR,构成了每个工作日最动听的乐章。

  • 高频次的团队活动:这也是从团队视角来看,CR 既然是讨论,那么就不仅仅是一个人的事,而是一种团队活动,一种高频次、高质量、低成本的极具性价比的团队活动。

2.3 文化层:众里寻他千百度,蓦然回首,那人却在,灯火阑珊处

最后是文化层,CR 既是传帮带文化的重要组成,又是工程师文化的日常体现。

  • 传帮带文化的重要组成:资深工程师 CR 初级工程师的代码,可以给予高频次、高质量的指导;初级工程师 CR 资深工程师的代码,可以欣赏、学习高手如何把玩代码,取其精华去其糟粕。

  • 工程师文化的日常体现:协作、高效、进取、影响力,这些在各大互联网公司的工程师文化中频频出现的关键词,无一不与 CR 紧密相连。不夸张的说,工程师文化香不香,就看 CR 做的好不好。

3 行:CR 高效之法

认识完 CR,我们再来探讨一下如何高效的进行 CR。在我看来,高效 CR 首先有赖于以下几个客观条件和主观条件。

客观上来看,和谐的工程师文化清晰的代码规范是高效 CR 的两块基石。所谓和谐的工程师文化,就是说团队对代码秉持开放的心态,不藏着掖着,以写好代码为荣,以写坏代码为耻,持续关注代码质量。而清晰的代码规范,一方面提高了代码的可读性,另一方面也统一了编码风格,极大的减少了不同代码风格对评审者注意力的干扰。

主观上来看,对评审者而言,第一要端正态度,保持谦卑的心态,人非圣贤孰能无过,择其善者而从之,其不善者而改之。第二要谨记评审的对象是代码,而不是人,你写下的每一条评审意见都应基于客观事实和数据,做到有理有据,不带个人情绪。

基于我多年 CR 的实操经验,结合Google Code Review Developer Guide,我整理了一些高效 CR 的最佳实践,供你参考:

  • 依据个人偏好每天固定几个时间段专门用于 CR,我的习惯是出门前和下班前。CR 耗费的脑力丝毫不亚于编码,甚至更高,CR 过程中需要高度集中注意力。清醒的头脑和无干扰的环境,是提出高质量的评审意见的秘诀。
  • 除了固定时间段,任务切换期间也是 CR 的好时机。
  • 每次 CR 尽量控制在 15~30 分钟以内,超过 30 分钟应休息一会。
  • 收到 MR 之后,先判断一下 MR 的性质,如果是 Bug Fix 类型的 MR,应尽快评审,如果是新功能 MR,则可以等待下一个 CR 窗口。
  • 从收到 MR 到 CR 结束,最长不要超过 1 个工作日。
  • 开始 CR 之前先要搞清楚 MR 要解决的问题背景。
  • CR 就像读书,先看目录(改动的文件列表),再精读重点章节(包含核心业务逻辑的代码),最后扫读剩余章节。
  • 如果改动的文件数量较多,可以打开 IDE,切换到源分支,方便在 CR 过程中随时打开相关代码进行阅读。
  • 评审核心代码时,如果发现严重问题,应立刻终止 CR,找 MR 提交者当面讨论。
  • 如果 MR 提交者对评审意见提出异议,评审者应找提交者当面讨论,避免在评论区互踢皮球。
  • 合并代码之前应确保所有评审意见都被妥善处理。
  • 记得点赞。CR 不是只能提意见,看到优雅的代码不要吝啬你的表扬。

4 小结

2019 年 Stack Overflow 的一份调查报告显示,超过 7 成的程序员会把 CR 当做日常工作的一部分,近 1/3 的程序员每周在 CR 上花费 2~3 个小时,还有 1/3 的程序员每周花费 4~5 个小时。心里默默算一下,你是在拖后腿还是领路者?如果你还没做过 CR,那么赶紧行动起来;如果你已经在 CR,很好,请继续保持。一花一世界,一叶一菩提,码中自有乾坤。CR,走起!

5 参考

最后编辑于
©著作权归作者所有,转载或内容合作请联系作者
  • 序言:七十年代末,一起剥皮案震惊了整个滨河市,随后出现的几起案子,更是在滨河造成了极大的恐慌,老刑警刘岩,带你破解...
    沈念sama阅读 219,753评论 6 508
  • 序言:滨河连续发生了三起死亡事件,死亡现场离奇诡异,居然都是意外死亡,警方通过查阅死者的电脑和手机,发现死者居然都...
    沈念sama阅读 93,668评论 3 396
  • 文/潘晓璐 我一进店门,熙熙楼的掌柜王于贵愁眉苦脸地迎上来,“玉大人,你说我怎么就摊上这事。” “怎么了?”我有些...
    开封第一讲书人阅读 166,090评论 0 356
  • 文/不坏的土叔 我叫张陵,是天一观的道长。 经常有香客问我,道长,这世上最难降的妖魔是什么? 我笑而不...
    开封第一讲书人阅读 59,010评论 1 295
  • 正文 为了忘掉前任,我火速办了婚礼,结果婚礼上,老公的妹妹穿的比我还像新娘。我一直安慰自己,他们只是感情好,可当我...
    茶点故事阅读 68,054评论 6 395
  • 文/花漫 我一把揭开白布。 她就那样静静地躺着,像睡着了一般。 火红的嫁衣衬着肌肤如雪。 梳的纹丝不乱的头发上,一...
    开封第一讲书人阅读 51,806评论 1 308
  • 那天,我揣着相机与录音,去河边找鬼。 笑死,一个胖子当着我的面吹牛,可吹牛的内容都是我干的。 我是一名探鬼主播,决...
    沈念sama阅读 40,484评论 3 420
  • 文/苍兰香墨 我猛地睁开眼,长吁一口气:“原来是场噩梦啊……” “哼!你这毒妇竟也来了?” 一声冷哼从身侧响起,我...
    开封第一讲书人阅读 39,380评论 0 276
  • 序言:老挝万荣一对情侣失踪,失踪者是张志新(化名)和其女友刘颖,没想到半个月后,有当地人在树林里发现了一具尸体,经...
    沈念sama阅读 45,873评论 1 319
  • 正文 独居荒郊野岭守林人离奇死亡,尸身上长有42处带血的脓包…… 初始之章·张勋 以下内容为张勋视角 年9月15日...
    茶点故事阅读 38,021评论 3 338
  • 正文 我和宋清朗相恋三年,在试婚纱的时候发现自己被绿了。 大学时的朋友给我发了我未婚夫和他白月光在一起吃饭的照片。...
    茶点故事阅读 40,158评论 1 352
  • 序言:一个原本活蹦乱跳的男人离奇死亡,死状恐怖,灵堂内的尸体忽然破棺而出,到底是诈尸还是另有隐情,我是刑警宁泽,带...
    沈念sama阅读 35,838评论 5 346
  • 正文 年R本政府宣布,位于F岛的核电站,受9级特大地震影响,放射性物质发生泄漏。R本人自食恶果不足惜,却给世界环境...
    茶点故事阅读 41,499评论 3 331
  • 文/蒙蒙 一、第九天 我趴在偏房一处隐蔽的房顶上张望。 院中可真热闹,春花似锦、人声如沸。这庄子的主人今日做“春日...
    开封第一讲书人阅读 32,044评论 0 22
  • 文/苍兰香墨 我抬头看了看天上的太阳。三九已至,却和暖如春,着一层夹袄步出监牢的瞬间,已是汗流浃背。 一阵脚步声响...
    开封第一讲书人阅读 33,159评论 1 272
  • 我被黑心中介骗来泰国打工, 没想到刚下飞机就差点儿被人妖公主榨干…… 1. 我叫王不留,地道东北人。 一个月前我还...
    沈念sama阅读 48,449评论 3 374
  • 正文 我出身青楼,却偏偏与公主长得像,于是被迫代替她去往敌国和亲。 传闻我的和亲对象是个残疾皇子,可洞房花烛夜当晚...
    茶点故事阅读 45,136评论 2 356