• 首页 > 
  • 业务中台Code Review

业务中台Code Review

Code Review可以有效提高整体代码质量,及时发现代码中可能存在的问题。本次最佳实践将为您总结和整理了一些如何做Code Review和Code Review使用的经验和技巧。

最佳实践综述

业务中台是具备业务属性并支持多种业务属性的共性能力组织,有助于业务的复用,对业务的快速响应能力。从广泛的意义上来说,一切的中台都是业务中台,它们源自业务并服务于业务。由于业务中台支撑的重要性,业务中台代码Code Review变得非常重要。

Code Review的价值体现于:

  1. 开发人员可以放心的发挥自己的创造性,因为他知道他有个坚强的后盾。

  2. 开发人员都会尽全力写最好的代码,不想当时就被别人找到缺陷。对软件的可维护性更加尽心。

  3. 极大的提高软件质量以及可维护性。

Code Review益处

Code Review是一件非常有益的事。其优势重点体现在以下几个方面。

代码团队易读

开发团队中,开发人员的水平互有高低,每个人侧重的领域也有不同。如何让高水平的开发帮助新人成长?怎么让大家都对自己侧重领域之外的知识保持了解?怎么样在有人离职后其他人能快速接手离职同事的工作?这些都是团队管理者关心的问题。

优秀的软件开发者为他们的代码编写恰到好处的注释。好的注释能够让代码更容易理解,更少的bug,更易更改。注释的方法有如下几种:

  1. 出现在方法、类之前的重要注释是一种规范。JAVA中,它通常以Javadoc注释的形式编写,以/**开头,并包含@语法,例如方法的@param和@return。

    /**
     * 存钱交易业务处理
     * @param userId
     * @param money
     * @throws ParameterException
     */
    @Override
    @TxcTransaction
    public void depositMoney(String userId, int money) throws ParameterException {…}
    
  2. 指定从其他地方复制或改编的一段代码的来源。

    /**
     * jdk1.5自带的并发库中Future类就能满足这个需求。Future类中重要方法包括get()和cancel()。get()获取数据对象,如果数据没有加载,就会阻塞直到取到数据,而 cancel()是取消数据加载。另外一个get(timeout)操作,表示如果在timeout时间内没有取到就失败返回,而不再阻塞。 
     */
    boolean bdStatus = false;
    
    final ExecutorService exec = Executors.newFixedThreadPool(1);
    Callable<String> call = new Callable<String>() {
    	@Override
    	public String call() throws Exception {
    		List<AccountInfo> list = userService.list();
    		return "true";
    	}
    };
    
    try {
    	Future<String> future = exec.submit(call);
    	// 设置超时时间10s
    	String obj = future.get(1000 * 10, TimeUnit.MILLISECONDS);
    	bdStatus = Boolean.parseBoolean(obj);
    	System.out.println("the return value from call is :" + obj);
    } catch (TimeoutException ex) {
    	System.out.println("====================task time out===============");
    	ex.printStackTrace();
    	bdStatus = false;
    } catch (Exception e) {
    	System.out.println("failed to handle.");
    	e.printStackTrace();
    	bdStatus = false;
    }
    // 关闭线程池
    exec.shutdown();
    
  3. 并不是每个语句都需要注释,阅读者至少是知道编程语言的用法。对于晦涩难懂的代码应该添加注释。

    // MT 模式的管理器
    @Bean
    public MTRelationShipManager mtRelationShipManager() {
    	MTRelationShipManager mtRelationShipManager = new MTRelationShipManager();
    	mtRelationShipManager.setInterceptorNames("mtBranchInterceptor");
    	mtRelationShipManager.setOrder(1);
    	mtRelationShipManager.setProxyTargetClass(false);
    	mtRelationShipManager.setBeanNames(new String[] { "com.taobao.txc.rm.mq.TxcMQProducerAdaptor" });
    	return mtRelationShipManager;
    }
    

代码审查,是一个很好的知识共享的方式。通过代码审查,高手可以直接指出新手代码中的问题,新手可以马上从高手的反馈中学习到好的实践,得到更快的成长;通过代码审查,前端也可以去学习后端的代码,做功能模块A的可以去了解功能模块B的。

有些高手觉得给新手代码审查浪费时间,自己也没收获。其实不然,新人成长了,就可以更多的帮高手分担繁重的任务;良好的沟通能力、发现问题的能力、帮助其他人成长,都是技术转管理或技术上更上一层楼必不可少的能力,而通过代码审查可以有效的去练习这些方面的能力。

提高代码质量

现实中的项目总是人手缺进度紧,所以被压缩的往往就是自动化测试和代码审查,结果影响代码质量,欠下技术债务,最后还是要加倍偿还。

也有人寄希望于开发后的人工测试,然而对于代码质量来说,很多问题通过测试是测试不出来的,只能通过代码审查。如代码的可读可维护性,代码的结构,一些特定条件才触发的死循环、逻辑算法错误等等。还有一些安全上的漏洞也更容易通过代码审查发现和预防。

举例代码

public static int dayOfYear(int month, int dayOfMonth, int year) {
    if (month == 2) {
        dayOfMonth += 31;
    } else if (month == 3) {
        dayOfMonth += 59;
    } else if (month == 4) {
        dayOfMonth += 90;
    } else if (month == 5) {
        dayOfMonth += 31 + 28 + 31 + 30;
    } else if (month == 6) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31;
    } else if (month == 7) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30;
    } else if (month == 8) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31;
    } else if (month == 9) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31;
    } else if (month == 10) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31 + 30;
    } else if (month == 11) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31 + 30 + 31;
    } else if (month == 12) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31 + 30 + 31 + 31;
    }
    return dayOfMonth;
}

重复代码是危险的,在两处有相似或者相同的代码,则可能两处都有bug,并且修复了一个地方的问题后可能忘记修复另一个地方。例如上述代码每个月的天数重复出现以及dayOfMonth+=重复出现多次。

改进方式

可以用数组存储天数,for while循环精简dayOfMonth+=。

团队代码规范

每个团队都有自己的代码规范,有自己的基于架构设计的开发规范。然而时间一长,就会发现代码中出现很多不遵守代码规范,或是绕过架构设计的代码。比如难以理解和不规范的命名、三层架构中UI层绕过业务逻辑层直接调用数据访问层代码等。

如果这些违反规范的代码被纠正的晚了,后面再要修改成本就会很高,而且团队的规范也会慢慢的形同虚设。而通过代码审查,就可以及时的去发现和纠正这些问题,保证团队规范的执行。

《阿里巴巴Java开发手册》是阿里巴巴集团技术团队的集体智慧结晶和经验总结,经历了多次大规模一线实战的检验及不断的完善,系统化地整理成册,反馈给广大开发者。

现代软件行业的高速发展对开发者的综合素质要求越来越高,因为不仅是编程知识点,其它维度的知识点也会影响到软件的最终交付质量。如:

  • 数据库的表结构和索引设计缺陷可能带来软件上的架构缺陷或性能风险。

  • 工程结构混乱导致后续维护艰难。

  • 没有鉴权的漏洞代码易被黑客攻击。

本手册以JAVA开发者为中心视角,划分为编程规约、异常日志、单元测试、安全规约、工程结构、MySQL数据库六个维度,再根据内容特征,细分成若干二级子目录。根据约束力强弱及故障敏感性,规约依次分为强制、推荐、参考三大类。对于规约条目的延伸信息中:

  • “说明”对内容做了适当扩展和解释。

  • “正例”提倡什么样的编码和实现方式。

  • “反例”说明需要提防的雷区,以及真实的错误案例。

阿里巴巴Java开发手册关于代码审查的好处还有很多,在本次实践中将不再一一赘述。Code Review和写自动化测试一样,都是属于磨刀不误砍柴工的工作,在上面投入一点点时间,未来会收获代码质量,会节约整体的开发时间。

Code Review怎么做

把Code Review作为开发流程的必选项

在很早以前,代码审查曾作为代码流程的一部分,但只是一个可选项,没有Code Review也可以把代码合并到master中。最终导致开发者不会每次交付代码都会去做Code Review,等到去检查的时候已经有了太多的代码变更,审查起来非常困难,而且就算审查出了问题,修改代码依旧十分困难。

现在对代码的审查则是作为开发流程的一个必选项,每次开发新功能或者修复Bug,开一个新的分支,分支要合并到master有两个必要条件:

  1. 所有的自动化测试通过。

  2. 有至少一个人Code Review通过,如果是新手的PR(Pull Request),还必须有资深程序员Code Review通过。

这样把Code Review作为开发流程的一个必选项后,就很好的保证了代码在合并之前有过Code Review。而且这样合并前要求代码审查的流程,好处也很明显:

  1. 由于每一次合并前都要做代码审查,这样一般一次审查的代码量也不会太大,对于审查者来说压力会较小。

  2. 如果在Code Review时发现问题,被审查者希望代码能尽快合并,也会积极的对审查出来的问题进行修改,不至于对审查结果太过抵触。

当Code Review难以推行的时候,不妨先尝试着把Code Review变成开发流程的一个必选项。

把Code Review变成一种开发文化而不仅仅是一种制度

把Code Review作为开发流程的必选项后,不代表Code Review这件事就可以执行的很好,因为Code Review的执行,很大部分程度上依赖于审查者的认真审查,以及被审查者的积极配合,两者缺一不可。

如果仅仅只是当作一个流程制度,那么就可能会流于形式。最终结果就是看起来有Code Review,但没有人认真审查,随便浏览后便评审通过,或者即使发现问题也不愿意修改。

如果要把Code Review这件事做好,必须让Code Review变成团队的一种文化,开发人员从心底接受这件事,并认真执行这件事。

要形成这样的文化,不那么容易,也没有想象的那么难,比如这些方面可以参考:

  1. 要让开发人员认识到Code Review这件事为自己、为团队带来的好处。

  2. 要有几个人做好表率作用,榜样的力量很重要。

  3. 对于管理者来说,你激励什么,往往就会得到什么。

  4. 像写自动化测试一样,把Code Review要作为开发任务的一部分,给审查者和被审查者都留出专门的时间去做这件事。

如何形成这样的文化,除此以外,还有很多方法可以尝试。但是只有真正让大家都认同和践行,才可能去做好Code Review这件事。

Code Review的经验技巧

选什么工具辅助做Code Review

现在很多源代码管理工具都自带Code Review工具,典型的像Github、Gitlab、微软的Azure DevOps,尤其是像Gitlab,还可以自己在本地搭建环境,根据自己的需要灵活配置。

配合什么样的开发流程比较好

像Github Flow这样基于分支开发的流程是特别适合搭配Code Review的。其实不管什么样的开发流程,关键点在于代码合并到master(主干)之前,要先做Code Review。

先设计再编码

有些新人发现自己的代码提交PR后,会收到一堆的Code Review意见,必须要做大量的改动。这多半是因为在开始做之前,没有做好设计,做出来后才发现问题很多。

建议在做一个新功能之前,写一个简单的设计文档,表达清楚自己的设计思路,找资深的开发者先帮你做一下设计的审查,发现设计上的问题。设计上没问题了,再着手开发,那么到Review的时候,相对问题就会少很多。

代码在提交Code Review之前,作者要自己先Review和测试

代码在提交Code Review之前,有一些非常明显的问题,有些甚至自己都没有测试过,就等着别人Code Review和测试帮助发现问题。这种依赖心理无论是对自己还是对团队都是很不负责任的。

一个好的开发人员,代码在提交Code Review之前,首先是要自己先Review一遍,把该写的自动化测试代码写上,基本的测试用例执行一遍。

对于团队提交的PR,有个要求就是要在PR的描述中增加截图或者录屏,就是为了通过截图或者录屏,确保提交PR的人自己是先测试过的。这也是一个有效的辅助手段。

总结

好的软件需要具备三个关键属性:

  1. 帮助开发者只需要在一处修改bug,使用清晰的注释记录和假设条件,能够防止其他程序员引入错误。

  2. Fail fast能够帮助尽早发现错误,避免全局变量能够帮助定位与局部变量有关的bug。

  3. Code Review是找到晦涩难懂的代码的有效方法,因为其他程序员阅读并尝试理解它。使用恰到好处的注释,一个变量一个目的,更好的命名,更好地使用空格都可以提供代码的可理解性。

Code Review是一种非常好的开发实践,如果你还没开始,不妨逐步实践起来;如果已经做了效果不好,不妨对照一下,看有没有把Code Review作为开发流程的必选项而不是可选项,有没有把Code Review变成一种开发文化而不仅仅是一种制度。