什么是CodeReview
CodeReview的目的是提升代码质量,尽早发现潜在缺陷与BUG,降低修复成本,同时促进团队内部知识共享,帮助更多人更好地理解系统。
如何进行CodeReview
如果使用Object-C进行开发,CodeReview分为两个阶段:本地 Review、远端 Review
本地Review
使用OCLint来进行本地走查,关于其安装和使用,参考文章:
OCLint 代码静态分析
自动化 Code Review
OCLint 安装与使用
OCLint 规则与结果分析
远端Review
远端 Review 和 本地 Review 大体相似,区别在与引用构建的脚本的对象从 Xcode 变成了 Jenkins CI ,报告的展示者从 Xcode 变成了 SonarQube 。
具体流程也在自动化 Code Review文章中可以找到。
以上过程可以说还是在自我检查阶段。
团队协作时的CodeReview
在团队(尤其是大规模团队)协作中,一般需要有人工Review的过程,如使用gitflow或其他协作工具,代码提交更新时,需要先经过CodeReview,通过后才允许合并,参考文章:
从零开始Code Review
Git 在团队中的最佳实践--如何正确使用Git Flow
关于CodeReview的一些原则
架构/设计/常规
1.单一职责原则
一个类只能干一个事情,一个方法最好也只干一件事情。比较常见的违背是一个类既干UI的事情,又干逻辑的事情,这个在低质量的客户端代码里很常见。
2.行为是否统一
例如:
1)缓存是否统一
2)错误处理是否统一
3)错误提示是否统一
4)弹出框是否统一
5)……
3.代码污染
代码有没有对其他模块强耦合
4.重复代码-->应该抽取
5.开闭原则
6.面向接口编程
7.健壮性
1)是否考虑线程安全
2)数据访问是否一致性
3)边界处理是否完整
4)逻辑是否健壮
5)是否有内存泄漏
6)有没有循环依赖
7)有没有野指针
8)是否检查了数组的“越界“错误
9)……
8.错误处理
9.改动是不是对代码的提升
新的改动是打补丁,让代码质量继续恶化,还是对代码质量做了修复
10.效率/性能
1)关键算法的时间复杂度多少?有没有可能有潜在的性能瓶颈
2)客户端程序对频繁消息和较大数据等耗时操作是否处理得当
代码风格
1.可读性
衡量可读性的可以有很好实践的标准,就是 Reviewer 能否非常容易的理解这个代码。如果不是,那意味着代码的可读性要进行改进
2.命名
1)命名对可读性非常重要
2)是否跟系统属性命名造成冲突
3)英语用词尽量准确一点,必要时可以查字典
3.函数长度/类长度
1)函数太长的不好阅读
2)类太长了,检查是否违反的 单一职责 原则
4.注释
恰到好处的注释,不是注释越多越好
5.参数个数
不要太多,一般不要超过 3 个
参考文章:移动端 iOS 年终工作总结
最后,我们分享一篇 Tumblr 工程师 cyle 进行代码 review 的经验(来自知乎的某个回答):
在 Tumblr,review 代码对于所有的工程师来说,都是一项十分重要的工作,甚至比写代码本身更重要。我们的代码仓库被数百人共享,所以,我们不仅要写最好的代码,还要尽量使写出来的代码易于理解。而这一切的关键因素就是花时间 review 别人的代码,保证一切都是有条不紊的进行。
在 Tumblr,代码的所有的改动都是通过在一个内部的 github 服务上提交申请(Pull request, 以下简称PR)的方式完成的。我们的代码仓库里面同时存放着 PHP 写的后端,数据库模式, swift/Obj-C 写的 iOS 应用,JAVA/Kotlin 写的 Android 应用,Go/C/C++ 写的基础架构,以及其他 Scala,Node.js,Python 或其他语言写的项目。我们所有的代码仓库都是由代码作者提交 PR,然后再由同事进行审阅,最后再合入到主分支并发布到生产环境对用户提供服务。
从我加入 Tumblr 以来,我个人 review 代码的方式发生了巨大的变化。以前,我主要是自己写代码然后在一小撮人里 review;现在在 Tumblr,庞大的代码仓库有者数百名贡献者,变化之大可想而知。幸运的是,我遇到了很多好的前辈。开始的时候我一个月 review 一个 PR,现在我平均每周需要 review 25个 PR。以下是我在 review 代码期间遵循的一些基本原则,这些原则使得我能够及时 review 代码,并能给他人带来帮助。你需要知道你在给谁 review 代码
在收到 PR 的时候,我关注的第一件事情是:「谁写的这段代码?TA 是初级工程师还是资深工程师?TA 是刚接触到这部分代码还是开发多年的老鸟?我以前是否 review 过 TA 的代码?我对这段代码所属的模块是否熟悉?」
当我 review 我熟悉的人的代码的时候,我十分清楚 TA 写这段代码的时候是怎么想的,并且我知道 TA 在写代码可能遇到什么问题。初级工程师的常常需要更多手把手的指导,比如为他们提供更多的样例代码和参考;资深工程师则需要提醒他们注意他们写的高性能,抽象或者讨巧的代码是很难阅读的,所以需要写更多的注释和文档。
另外,在对 review 的代码写评论的时候,也需要注意让除了提 PR 的作者本人之外的人能够看懂。因为:
- 有些人是通过别人review代码的评论学习的;
- 对初级工程师而言,这是帮助他们理解Turmble代码复杂性的方法。在六个月之内,你很可能会再次阅读这段代码,并理解它是如何运作的,如果有一段有意义的 code review 评论在那里,你也就更容易知道当初这段代码为什么这么写,以及他是如何工作的。
review 代码的时候也需要想着其他人
我 review 代码的关键在于,无论谁提交代码改动的时候,都需要知道这段改动本身的的含义以及做出这写改动的动机和上下文。对我来说,理想情况是任何一个人看到这个 PR,都有足够的上下文去理解相应的改动,TA 能够知道这段改动为什么要这么写以及它究竟是如何工作的。特别是陈年的老代码和共享的代码仓库,也许三年之后,有人还会需要从这个 PR 去理解它做了什么。如果做不到上面说的需求的话,或者这次 PR 里面甚至都没有任何的上下文信息,那就一定是有问题的。细节总归是越多越好。
我从来不担心代码风格或者语法的问题,因为我们有自动化的流程来保证新代码或者改动是符合我们的代码要求的。正如我在我是如何写代码的说的,我需要的有清楚的文档(随代码的注释和专门的文档)的,易于理解(哪怕不是那么讨巧)的的代码。我宁愿阅读有十倍代码量但是易于理解的代码,而不是仅一行的复杂难懂的多重嵌套,特别是这段代码的作者已经在这里堵了好几次,并且被那些陈年的,没有文档的“聪明"代码折腾得要死的时候。
我觉得我能够理解这次代码的改动之后,我还会从一个不怎么了解这部分代码的人的角度重新审视它(因为下次我也可能 review 不了解的代码),并思考如何如何使得这份代码更加清晰。我会试着从一个刚入职六个月的新手的角度来看这段代码,并理解它是如何工作的。懂得 PR 的影响面
很多时候单个的 PR 并不能把所有的事情工作的完成。在 Tumblr,我们总是试图让每个 PR 都尽可能的小,那样的话我们就能够快速且安全的进行代码 review,而不必要攒到 5000 行改动才进行 review。因此,一些大的工作需要被分解成小的片段,有些 PR 负责构建基础的组件,有些 PR 则是完成了基础实现之后做的。
换句话说,一些代码常常有一些待完成的事项或者以后需要修正的点,那么在代码里面标注好TODO
,写下待办事项的名字和具体的工作,就是十分常见的行为了。这样,我们就不用等到完成了所有的代码之后再提交这次 PR 了。关注整个代码 review 的过程
帮助我及时 review 代码,并且跟进整个 review 过程的主要工具是:邮件。我会检查我收到的每一份 github 邮件,我并不保证仓库里面的所有改动都会通知到我,但是我会检查我收到的任何一封跟 PR 相关的邮件。我就是依靠这种方式跟进整个 reivew 过程的,因为这个来回理想情况下不会超过一整天。
在 Tumblr,大多数的 reviewer 是自动的在 PR 作者里面轮流选出来的。摊派任务的时候会给我发送一封邮件,并把跟这个PR相关的所有信息都和我关联起来。自此以后,我就需要一直关注我的邮件,并保证我不仅要抽时间尽快 review 代码,还需要跟进这次 PR 中作者对我提的评论进行的对应的修改。记住你是一个「人」
无论是在 review 代码还是写代码的过程中,必须要始终记得一件事情:你是一个「人」,并且你 review 的代码的作者也是一个「人」。
通过 review 代码让他们觉得受到质疑对你们都有好处。在你写建议,有问题或者发现 TA 没注意到一个边界条件的时候,保持友善的态度。即使是写了多年“完美无缺”代码的老鸟,有些时候也应该把他们当作会犯错误的普通人来看待。即使那些与你共事很久的人并不在意你拿他们开玩笑,总会有新人不理解的。
记住,被很多人分享的,并且在不断更新的代码必然往往是奇怪的、复杂的,特别是那些已经存在了十多年的代码。记住,很多时候工作是十分仓促的,你要做的就是在有限的时间完成尽可能优质的代码。我们并不能为了完美的代码耽搁项目进度,但是我们需要保证我们写的每一份代码尽可能的好,无论我们在写代码还是在 review 代码。