【转】code review 重要性

代码评审的 8 点建议

学校有一点没有教你的是:如何进行代码评审。你学习了算法、数据结构,以及编程语言基础,但没有人坐下来说:“这是一些能让你提出更好的反馈的办法”。

代码评审是编写良好软件过程中的关键步骤。代码评审在于尽可能使得其具备高质量且 bug 少的特点。良好的代码评审文化也会带来其他收益:你减少了产生 bug 的因素;同时代码评审也是培养新成员和分享知识的良好途径。

假设

阅读本文之前,有必要作出几点假设,如下:

  • 你在一个受信任的环境中工作,或者你和你的团队正在改善你的信任度。
  • 你可以在非代码环境中提出反馈,也可以在你的团队中提出反馈。
  • 你的团队希望产出更好的代码,也理解 perfect 是一个动词而非形容词。我们可能会为明天的工作找到更好的解决方案,同时我们需要保持开放的心态。
  • 你的公司注重代码的质量,并且理解高质量代码或许无法快速“上线”。这里引用“上线”是为了说明:很多时候未经测试和评审的代码实际上可能不起作用。

有了上述假设条件,接下来让我们进入正文。

1. 我们是人类

要知道其他人在你将要评审的代码中投入了很多时间,他们也想让代码质量更高。你的同事(通过代码)努力地表达自己的意图,谁也不想写出蹩脚的代码。

保持客观是很困难的。请确保总是评判代码本身,并试着去理解上下文的含义。尽可能减轻评判带来的不良影响。不要说:

你写的这个方法令人费解。

尝试换个说法以针对代码本身,并增加你的解释:

这个方法有点不好理解,我们是否可以为这个变量起一个更好的名字呢?

这个例子中解释了我们作为读者时对代码的感觉,这关乎于我们自己以及我们对代码的解释,而与编写者的编码方式或意图无关。

每个的 Pull Request 都有它本身的高难度交流。尝试与你的队友达成共识, 共同努力以实现更好的代码。

如果你刚刚认识一名团队成员,并且针对某个 Pull Request 有一些重要反馈,请共同浏览一遍代码。这将是发展同事关系的一个好机会。以这种方式与每个同事合作,直到你不再感到难为情。

2. 自动检查

如果计算机可以决定并执行一条规则的话,那就让计算机完成它。争论应使用空格还是 tabs 属于浪费时间。相反,应把时间花在制定规则上并且达成一致。这也是观察团队如何在低风险情景下处理“反对还是提交代码”的机会。

编程语言和现代工具流不缺乏执行规则(的辅助检查程序)并反复应用它们的方法。在 Ruby 中,有 Rubocop;在JavaScript中,有 eslint。找到语言这类辅助检查程序,并将其嵌入到构建流中。

如果你发现现有的辅助检查程序存在不足,那么可以自己编写!定制规则相当简单。在 Gusto 中,我们使用定制的辅助检查规则来捕获类的废弃用法,或者适当地提醒人们遵守某些 Sidekiq 最佳实践。

3. 全员评审

听起来,把全部的代码评审工作交给 Shirley 是一个好主意。

Shieley 是一位大牛,她总是知道如何有效编程。她清楚系统的输入输出,在公司呆的时间比团队成员的总和都要长。

然而对于某些事情,Shirley 理解它并不代表其他团队成员也理解了。评审 Shirley 的代码时,年轻的团队成员或许会在指出某些问题时犹豫不决。

我意识到将评审工作分配给不同的成员会产生有益的的团队动力和更好的代码。一名初级工程师在某次代码评审中作出的最有力的评论是:“我看不太懂。”这是使代码变得更加清晰简单的机会。

在团队中推广代码评审。

4. 保持可读性

Gusto 中,我们使用 GitHub 管理我们的项目。GitHub 中的每个 <textarea> 都支持 Markdown,这是一种在注释中添加 HTML 格式文本的简单方法。

使用 MarkDown 是一种增加内容易读性的方式。GitHub 及你选用的工具可能会具备语法高亮功能,这对代码片段的阅读非常友好。使用一对反引号 (`) 嵌入代码或三个反引号 (```) 增加代码块,带来更好的交流体验。

善于利用 Markdown 语法(尤其当你写的代码包含注释时)。这样做将有助于使你的评论内容具体且重点突出。

5. 至少反馈一条正面评价

代码评审本质上是带有消极影响的事情。在我把代码发到网上前,可以告诉我这个代码有什么问题。这就是代码评审应该干的事情。开发者投入时间编写代码,同时希望你能指出如何能够做得更好。

为此,总是应该给出至少一条正面评价,并且使其富有意义和充满人情味儿。如果有人最终解决了长期攻关的问题,请无保留地表露出兴奋,它可以是简单的一个 👍 或者 “赞一个”。

在每次的代码评审中留下正面评价会微妙地提醒我们在一起共事。如果我们生产良好的代码,我们都将受益。

6. 提供替代方案

我尝试去做的一件事是:用替代方案来实现(相同的功能),尤其是刚刚开始学习一种编程语言和框架的时候。

谨慎一些。如果表述不恰当,可能会让人觉得你傲慢或自私:“这是我实现的方式。”尽量保持客观,并讨论你所提供的备选方案的优缺点。如果你的方案很棒,将有助于拓展每个成员的技术视野。

7. 延迟是关键因素

快速修正代码非常重要。(下面的规则会使它变得更容易:保持小代码量。)

长时间地延迟代码评审会降低生产力和斗志。被分配去评审 3 天前的 PR 会让人感到不舒服。噢,的确如此。我究竟在干什么?反复地在上下文构建环境中切换。要纠正这一点,你需要提醒你的团队,进度依赖于整个团队而非个人。促使你的团队关注代码审查的延迟情况,并把它做得更好。

如果你希望减少自己的代码评审延迟,我建议遵循这条规则:编写任何新代码之前,首先评审代码。

作为一种直接处理延迟的策略,尝试在代码评审时进行配对。找一个结对编程工作台,或者共享屏幕来浏览和评审代码。生成解决方案时采用配对方式,使大家都赞成它。

8. 对提 pr 者的忠告:保持小代码量

在一次代码评审中,你收到的反馈的质量与 Pull Request 的代码量成反比。

为作出令人信服且有建设性的反馈,要知道更小的 Pull Request 更易于阅读。

如果你保持 Pull Request 足够小(避免 The Teeth)(译注:原文中用牙齿的大小类比代码块的大小,如果牙齿太大则可能会戳破皮肤,同理,代码块也不宜太大),你将需要结合上下文进行更大范围的交流。这个 Pull Request 如何合入本周本月的工作中?我们下一步要做什么,以及这个 Pull Request 是怎么推进工作的?诸如白板编程和面对面讨论这些形式的讨论非常重要。更小的 Pull Request 很难让人记住它的上下文。

不同的编程语言和团队对“小”有不同的定义。对我而言,我尽量保持 Pull Request 少于 300 行代码。

结论

希望这 8 条建议能够帮助你和你的团队作出更好的代码评审。通过改进你们的代码评审流程,你可以收获更好的代码、更融洽的队员,以及更好的业务发展。

原文地址 : https://github.com/xitu/gold-miner/blob/master/TODO1/8-tips-for-great-code-reviews.md

 

Code Review 主要Revivew什么

Architecture/Design

  • 单一职责原则.

    • 这是经常被违背的原则。一个类只能干一个事情, 一个方法最好也只干一件事情。 比较常见的违背是一个类既干UI的事情,又干逻辑的事情, 这个在低质量的代码里很常见。

  • 行为是否统一

    • 比如缓存是否统一,错误处理是否统一, 错误提示是否统一, 弹出框是否统一 等等。

    • 同一逻辑/同一行为 有没有走同一Code Path?低质量程序的另一个特征是,同一行为/同一逻辑,因为出现在不同的地方或者被不同的方式触发,没有走同一Code Path 或者各处有一份copy的实现, 导致非常难以维护。

  • 代码污染

  • 重复代码

    • 主要看有没有把公用组件,可复用的代码,函数抽取出来。

  • Open/Closed 原则

    • 就是好不好扩展。 Open for extension, closed for modification.

  • 面向接口编程 和 不是 面向实现编程

    • 主要就是看有没有进行合适的抽象, 把一些行为抽象为接口。

  • 健壮性

    • 对Corner case有没有考虑完整,逻辑是否健壮?有没有潜在的bug?

    • 有没有内存泄漏?有没有循环依赖?(针对特定语言,比如Objective-C) ?有没有野指针?

    • 有没有考虑线程安全性, 数据访问的一致性

  • 错误处理

    • 有没有很好的Error Handling?比如网络出错,IO出错。

  • 改动是不是对代码的提升

    • 新的改动是打补丁,让代码质量继续恶化,还是对代码质量做了修复?

  • 效率/性能

    • 客户端程序 对频繁消息 和较大数据等耗时操作是否处理得当。

    • 关键算法的时间复杂度多少?有没有可能有潜在的性能瓶颈。

其中有一部分问题,比如一些设计原则, 可预见的效率问题, 开发模式一致性的问题 应该尽早在Design Review阶段解决。如果Design阶段没有解决,那至少在Code Review阶段也要把它找出来。

Style

  • 可读性

    • 衡量可读性的可以有很好实践的标准,就是Reviewer能否非常容易的理解这个代码。 如果不是,那意味着代码的可读性要进行改进。

    • 命名对可读性非常重要,我倾向于函数名/方法名长一点都没关系,必须是能自我阐述的。

    • 英语用词尽量准确一点(哪怕有时候需要借助Google Translate,是值得的)

  • 函数长度/类长度

    • 函数太长的不好阅读。 类太长了,比如超过了1000行,那你要看一下是否违反的“单一职责” 原则。

    • 恰到好处的注释。 但更多我看到比较差质量的工程的一个特点是缺少注释。

  • 参数个数

Review Your Own Code First

  • 跟著名的橡皮鸭调试法(Rubber Duck Debugging)一样,每次提交前整体把自己的代码过一遍非常有帮助,尤其是看看有没有犯低级错误。

如何进行Code Review

  • 多问问题。多问 “这块儿是怎么工作的?” “如果有XXX case,你这个怎么处理?”

  • 每次提交的代码不要太多,最好不要超过1000行,否则review起来效率会非常低。

  • 当面讨论代替Comments。 大部分情况下小组内的同事是坐在一起的,face to face的 code review是非常有效的。

  • 区分重点,不要舍本逐末。 优先抓住 设计,可读性,健壮性等重点问题。

Code Review的意识

  • 作为一个Developer , 不仅要Deliver working code, 还要Deliver maintainable code.

  • 必要时进行重构,随着项目的迭代,在计划新增功能的同时,开发要主动计划重构的工作项。

  • 开放的心态,虚心接受大家的Review Comments

原文地址 : https://juejin.im/entry/568a35ab60b2b60f65fb5d2e

posted @ 2018-10-26 14:35  GreenForestQuan  阅读(647)  评论(0编辑  收藏  举报