前些天群里分享了一份笔试题,其中有个问题是问如何进行 Code Review 的。我在群里调侃性地回答说:以我两年的 code review 经验,我可以告诉大家,我就是看其他人的代码风格是否和我的一致,一致就accept,不一致就refuse。
后来想想那近两年里我做的,简单地总结一下的话,似乎真如此。
我有较强的代码洁癖,并且在代码上表现出明显的强迫症。还在大学的时候,受团队师兄的影响,会比较注意格式。后来又译读了 Oracle 出的 Java 代码规范,并在进入上一家公司时积极推行代码规范的建立与实行,所以基本上,虽然我在规范上注明团队成员可参与修订,但是也没有其他人在维护,所以我在职的那段时间里,深圳研发部里的 Java 代码规范和 Android 代码规范,基本上也等同于“我”的代码规范了。因而我说,我的 code review 就是看其他人的代码风格与我的是否一致,事实上也接近如此。
那时我们搭建了 Phabricator 来进行 Code Review,为不影响开发效率,我们做的是提交入库之后的 Code Audit,每一个人提交的代码都需要至少一个人去审批才算通过(虽然理论上两个人更佳,但是老大太忙,而我们每人负责多个项目,平均每天的提交量也不少)。每天我都会尽量抽出一些时间,把提交的代码都检查一下。这种方式所看到的是每次提交的 diff 内容,所以通常而言,我也就是看规范,看命名,看方法内逻辑,有时会思考代码是否可以进一步优化。
看规范,自然是我所确立下来的代码规范了,这不多言。受《代码整洁之道》影响,我很推崇代码的自解释性,因此对方法、变量、类名等的命名要求会更严格。以我刚才看到的一段代码(https://github.com/qustgah/status-bar-compat/commit/23037e7b55e66f4fdaef59feb351ea83405e6946?diff=unified )作为例子,其中部分代码如下:
1 | /** |
这两个新增的方法命名在我看来问题就很大。首先是enableStatusBar(Activity activity)
方法,从注释和实现上来看,我们知道它是想实现根据头部控件颜色来设置状态栏颜色,而不是启用(enable)或禁用(disable)状态栏,因此这个方法名没有正确地描述它的行为。其次是getHeaderColor(Activity activity,View view)
方法,从实现上我们知道它是第一个设置了ColorDrawable
为背景的控件,并把这个颜色设置为状态栏颜色。先不论这种方法是否严谨,从命名习惯来看,getXXX
应该是获取,getHeaderColor()
给我的感觉应该是获取头部控件的颜色,它更像是一个幂等函数,而不是去做修改或设置的行为,所以如果要对状态栏进行设置,这里应该命名为setXXX
更恰当一些。这就是我对命名的一些看法。
进一步地,我会思考代码逻辑上的优化性,这里的优化不是指一定要使代码更少,执行效率更高,而是首先在可读性上的优化。在我看来,除非代码场景上有必要牺牲可读性来提升执行效率,否则撇掉可读性只一味追求效率是不利于团队项目的长期维护的,特别是年年有人员流动的公司项目。而实际实践下来,我发现照顾到代码的可阅读性的同时,对代码效率也没产生多少不良的影响。
在上面的代码中,若是由我来实现,我大概会定义一个方法,叫setHeaderColorToStatusBar
或setStatusBarWithHeaderColor
吧,当然这里的颜色叫BackgroundColor
可能会更准确一些,因为Header
除了有背景颜色,也有文字颜色。这个方法就是实现把状态栏颜色设置为头部的颜色。然后它的实现会是这样:
1 | View contentView = activity.findViewById(android.R.id.content); |
这里是随意选取例子,详细实现不多描述了。我在看代码时,会思考代码给人的直观感受是不是易于阅读的,它的逻辑性是否清晰。比如上面我修改的那部分代码,它就是获取contentView,然后从它获取到头部颜色,再设置进去。整体上改进了,再去考虑细节上的改进,比如前面的循环递归中的优化。
这种直观感受除了代码逻辑实现上的感受,也包括代码组织上的感受。最简单的一个例子就是空行的利用。
我们在定义一堆变量时,用空行,把静态变量与成员变量格开,把成员变量中有逻辑关联的变量与其他变量隔开,都有利于对代码逻辑的快速理解。
除了空行,还包括方法的排序。把相关联的代码放在一起,而不是根据字母或实现时间来排序。以及,把静态方法放在所有成员方法之后,而不是夹在成员方法之间或成员方法与变量之间。
简而言之,我们进行 Code Review,并不是为了要提前发现并解决 bug,事实上对 diff
的审核,也不能真正地发现更多的 bug,而在小团队快速迭代的过程当中也做不到每个人都对其他人的项目都了如指掌,对项目的熟悉不足必定难以发现功能逻辑上的问题。那么我们还为什么去进行 Code Review 呢?在我看来,其实是为了更好地实践代码规范,组织更清晰的逻辑,编写更整洁的代码,因为只有良好的代码规范,清晰的逻辑,整洁的代码,才恰恰能更多地避免 bug 的引入。