这周在公司一直给同事做CodeReview,感觉还是比较痛苦的,因为一些机制并不是很人性化,至少说,流程上有一些不成熟的环节。和大家一起分享,希望有经验的朋友给我一下批评建议。
方案1:Review Meeting,即Online Review。
基本不可行。很大程度上依赖于主持者的素质,而且如果主持者不积极,就会流于形式。此外时间成本过高,为reviewer+owner的时间总和。倒是可以将其改造为Team内部新技术新观点的交流Meeting,完全头脑风暴式的那种。
方案2:Offline Review
我特别想说的就是这种。包括重构、代码逻辑、代码规范。
对于变更的代码,owner应该提供一份ChangeList,基于需求或者功能点,列举出增删改动了那些类和方法以及关键变量,这就迫使owner首先review一下自己的代码,从而首先自身发现一些问题。对于代码重构,更需要这份清单,甚至是一个变更前后的类之间的关系图(当然可以从之前的LLD中复制过来)。额外的一个好处就是,重构会把很多方法原封不动的从一个类迁移到另一个辅助类,那么,reviewer就不用去care这些代码,而只是关心整体的结构,从而对症下药,对架构而不是代码,提出新的见解。
Code-review并不应该局限于纸面上,仅仅依赖于一个DB是不够的。一定要进行face to face的沟通或者by phone。由reviewer来讲解自己的心得,可以不受owner的主观意向影响,从其他角度来思考这个问题,还有就是迅速掌握代码,可以在owner离开的情况下迅速接手这段code;由owner来讲解,可以在介绍过程中,及时发现一些无法自圆其说的地方,然后加以修正。因此code-review从准备到完成的时间,应该大致为coding的50%左右。
最后,非常不赞同连错别字都写进code-review报告的行为。一方面要肯定reviewer的勤恳,但是,另一方面,这些与code无关的suggestion,会转移我们的视线。我们究竟要关注什么?是代码质量,是架构设计,而不是单词拼写错误。过多此类的suggestion,会把真正的问题掩藏起来,即使有critical等级的区别,也无济于事。一种极端的解决方式是禁止提这样的suggestion,比较缓和的方式是私下交流这些小错误。
额外需要指出的是,对代码规范的审核,尽量不要依赖人力,而是通过先进的工具来处理。人,总是要做一些机器做不了的事情,比如说重构与算法的review。这样,我们就可以有更多的时间focus在这些高层次的地方了。
补充:版本变更的代码比较(我们公司使用的是ClearCase)
如果类中原先有两个方法A和B,A在B的前面。版本变更后将A方法挪到了B方法的后面,那么ClearCase会只认为B方法是不变的,而认为新版本在B方法前删除了A方法,而在B方法后又添加了这个A方法——这就由ClearCase的逐行比较算法导致的,它毕竟只是一个文件控制工具,而不是for code file的;但是对于人而言,其实是没有改变的。这就对Code-Review添加了困扰,如果代码文件2万行,将一个方法从头位置挪到了尾部,这无疑就给reviewer造成了麻烦。
解决方案:如果ClearCase比较不同版本的代码文件的算法,能够细化为先list出一个类所有的成员(使用反射),按字母顺序排列,那么比对两个版本的类文件时,就可以按照成员的顺讯,先比较成员是否有所增删改变,再深入到方法属性中,用ClearCase原先的算法,逐行比较代码变更。
此外,还要注意嵌套类,因此要使用迭代来实现以上新算法。还有就是partial分散类的问题,这应该在代码规范中,禁止使用这种类的实现(自动生成的winform窗体除外)。