我们该如何做好Code-Review?

发表于   |   分类于

引言

午后的阳光，静静地照在你的脸上。这时候配上一杯82年的java，脑子一片灵光闪过，呃……上午刚写完需求，下午好像没什么事了,不如看看自己写的代码？至此，一场Code Review也就拉开序幕了。

前言

Code Review，即代码审查，是指对计算机源代码系统化地审查，常用软件同行评审的方式进行，其目的是在找出及修正在软件开发初期未发现的错误，提升软件质量及开发者的技术。代码审查以不同的形式进行，例如结对编程、非正式的看过整个代码，或是正式的软件检查。[1]

配对review(Mutual Review)

在我们项目组的code review中，制定了A童鞋与B童鞋的一对一review关系，“配对”成功后，这两位童鞋的代码负责相互review对方的代码，这样也形成了责任关系，可能这里有些人会表示并不赞同这种“强制”的责任制，更喜欢随意的工作方式。但是我们在实际中发现，如果不采取一对一的这种方式，很容易造成漏掉review某些童鞋代码。当然，在相互review之后，还可以继续对其他同事代码进行review，这样既确保了完备性，也保证了多样性。

流程和步骤如下：
Owner(被review的童鞋)，Reviewer(执行代码review的童鞋)。

  • Reviewer对Owner的代码进行review,针对某个缺陷代码提出review，格式如下：
1
2
3
4
5
6
7
8
@Override
  protected void onDestroy() {
      super.onDestroy();
      BitmapCache.getDefault().clear(true, false);
      BitmapCache.getDefault().close();
      //TODO review by xxx 2016/12/28 最好不要显示调用这个方法，如果必须要进行垃圾回收建议与runFinalizationSync()，否则调用可能不起作用。
      System.gc();
  }
  • Owner修改之后，并不要删除todo，而是由对应的Reviewer验证之后来删除，所以Owner修改完代码之后，格式如下：
1
2
3
4
5
6
7
8
@Override
  protected void onDestroy() {
      super.onDestroy();
      BitmapCache.getDefault().clear(true, false);
      BitmapCache.getDefault().close();
      //TODO review by xxx 2016/12/28 最好不要显示调用这个方法，如果必须要进行垃圾回收建议与runFinalizationSync()，否则调用可能不起作用。
      // FIX by xxx: 2017/1/11 
  }
  • Owner对已经修复完的问题进行再次Review，验证通过之后可以删除上述的todo及fix，如验证失败则继续补充相应的TODO。
1
2
3
4
5
@Override
  protected void onDestroy() {
      super.onDestroy();
      BitmapCache.getDefault().clear(true, false);
      BitmapCache.getDefault().close();

至此，Mutual Review也就顺利完成，这项工作我也把它称为Daily Review，因为这项review属于日常性，长期性的工作，从我们项目组来看，这项工作发现大多数缺陷代码。那么既然如此，为什么还需要Review Meeting呢？主要考虑有如下几点：

  1. Mutal Review的结果不能达成一致时，也就是说两个童鞋谁也不认同谁，这样就需要大家一起讨论来定夺应该怎么修改。
  2. Code Review会议能够集思广益，往往经过讨论之后能得到问题的最优解或次优解。
  3. Code Review会议使得各个人员之间能够面对面的进行交流，比起Mutual Review能对需要修改的点进行更细节的讨论，并且沟通效率更高。

代码评审会议(Review Meeting)

review meeting
代码评审会议一般流程如下：

  1. 每期版本迭代上线前至少进行一次，且最好应在上线前一周做完，最少也得提前三天，因为需要预留给测试时间进行review修改后的测试。
  2. 每次会议时间不宜过长，如讨论内容较多可分次举行。
  3. 在一次会议过程中，每位开发童鞋轮流对自己已经在代码中写过//TODO review by xxx 的地方进行讲解。讲解每处review的原因,其他童鞋可以进行讨论，是否有修改的必要？有没有更好地实现方式？是否需要将该案例加入到编码规范中？等等。
  4. 讨论结束后，总结会议中讨论的结果，并综合考虑修改的工作量与上线前剩余的时间，最终决定是否在上线前修改完成，还是可以考虑上线后下个版本进行修改。

结语

经过理论及实践表明，定期进行Code Review有如下几点好处：

  1. 能够学习他人代码，能够开阔思路，并且提升代码健壮性，改掉边界条件考虑不周的情况。
  2. 对于测试同学没有能够测试到的bug提前进行修复，降低线上bug及crash率。
  3. 在code review会议中集思广益，促进团队成员交流，有助于营造团结协作的团队氛围。

说了这么多，如果你不想看或者实在做不到的话只能送你一句话了。

参考资料

  1. Kolawa, Adam; Huizinga, Dorota. Automated Defect Prevention: Best Practices in Software Management. Wiley-IEEE Computer Society Press. 2007: 260/426. ISBN 0-470-04212-5.