最近做code review的5点经验分享

背景

最近两周项目代码提交到我这里来review的每周有20多个commit。也和同事聊天一起探讨了一下，几点自己认为需要注意的地方拿出来探讨一下。

以尽量小的单元提交push

其实我review代码的时候比较开放。有些仁者见仁的风格问题，除非统一内部讨论定下来，我是会放过的。比如有些人喜欢用lambda表达式，有些人喜欢用传统的for each。风格不同，没有必要让别人按照自己的想法来。

但是有一点哈，每次提交到负责review人那边的代码尽量一次不要太多。可以将一个开发功能事先拆分好。每次以不超过2个新增类。除了全局替换的，共不超过5个修改类为宜。或者提交的代码功能单一。一些代码优化，删除废弃代码什么的至少要在修改内容里标出。最好单独提code review申请。

为什么要这样呢？因为一次提交的代码太多了。从code review者的角度来讲：看代码的时候很可能联系不起来到底是出于什么功能修改的，一些问题可能会被漏掉。降低code review的质量。从开发者的角度来讲，一次开发考虑太多的东西，很可能每个都没有考虑细致到位。思路相对也没有那么清晰。在写代码被打断的时候，可能会中断其中一点的思考，一些要改进的漏掉了。相对是危险的操作。

lombok的callSuper属性

这个是一个自己有没有思考的问题。拷贝别人的代码的时候要想一想。下面的代码大家想一想要不要cullSuper？

@Setter
@Getter
@ToString(callSuper = true)
public class User implements Serializable {
    private Integer id;
    private String name;
    private String password;
    private Integer age;
}

实际运行结果如下：这时候User的super类的toString方法调用的object的toString就是打印类的全限定名@内存地址。对我们来说没有实际的参考价值，不建议用callSuper。

大家再思考下下面呢？

@Data
public class Pojo {
    private String value;
    @NotBlank
    private String detail;
}


@Setter
@Getter
@ToString(callSuper = true)
public class User extends Pojo{
    private Integer id;
    private String name;
    private String password;
    private Integer age;
}

这里的callSuper父类也是一个用户自己定义的类，它的属性是我们需要了解的，callSuper一点问题也没有。

业务代码建议定义非RuntimeException

一般我们使用的jar包中已经定义好了基本足够用的异常类型，我们可以直接拿来用。但是自定义异常也经常被见到。而且它们被使用通常只有一种情况：要标识出这个异常是某个特定的服务或者功能抛出的。

那为什么要用非RuntimeException呢？在《Java异常处理总结》这篇文章中有提到。RuntimeException是非检查异常，其他Exception是检查异常。非检查异常不需要显示处理，而检查异常会提示要try catch或者throw不然编译不通过。

非检查异常一般用于开发者小心一点就可以避免的异常，比如除以零，操作异常这样的。对于业务开发来说，这种异常何必让系统处理呢，自己不除以零就好，正常操作就好。如果定义为非检查异常，有些需要用户自己处理的异常可能会被漏掉了。所以不建议。

给客户端用的接口自己吞掉异常要小心

从代码架构上来讲，一个服务，如果分成给前端用的接口模块、给其他业务RPC用的接口模块、后台服务接口模块的话。前端用的接口模块我通常是自己处理异常不向外抛出。而所有的RPC接口都是明确抛出一个自定义异常的。

之所以这样是因为如果后端是java的话，前端一般是js等，异常的交互很不友好。前端处理很麻烦。而RPC接口，调用端也是后台语言。调用端是处理异常可以获取更详细的异常信息。

举个之前遇到的异常的例子：

之前一个获取秘钥的接口，在获取秘钥失败的时候改了逻辑。没有抛出异常而是返回空字符串。调用端将其作为正常结果处理。结果有次这个接口出问题了。调用的服务正常启动，需要用秘钥来进行后续处理的地方全都异常了。

如果这个地方，秘钥接口出错是抛出异常。因为当时获取秘钥是启动时加载。那么发版时这台机器就会无法启动。错误立即暴露出来。而线上发布都是低峰期灰度发布。所以几台机器没有启动没有影响。

java接口定义不要加任何修饰符号

阿里巴巴编程规范里有写：

推荐阅读

阿里巴巴编码规范（Java）证明

java的Closeable接口和Cloneable接口

JAVA数据处理的常用技术