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

清疚 2022-12-23 07:22 100阅读 0赞

**背景**

最近两周项目代码提交到我这里来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。

![format_png][]

大家再思考下下面呢？

@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一点问题也没有。![format_png 1][]

**业务代码建议定义非RuntimeException**

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

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

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

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

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

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

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

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

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

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

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

![format_png 2][]

**推荐阅读**

## [阿里巴巴编码规范（Java）证明][Java 1] ##

## [java的Closeable接口和Cloneable接口][java_Closeable_Cloneable] ##

## [JAVA数据处理的常用技术][JAVA] ##

[format_png]: /images/20221120/9f25b956efd041c6b65f597325a29f0b.png
[format_png 1]: /images/20221120/00a7143da5ca458d9e5cf64cb2c1d6d6.png
[Java]: http://mp.weixin.qq.com/s?__biz=MzUzNjAxODg4MQ%3D%3D&chksm=fafdec03cd8a6515f61cf42d3ce94ee7bc465f445cb61d4644d259c55a661cce4ab1a6dc4738&idx=1&mid=2247485101&scene=21&sn=c95f1a3fa03af199c78d59bb29ea411c#wechat_redirect
[format_png 2]: /images/20221120/cac654f6b4894daca4441c1642f2f58f.png
[Java 1]: http://mp.weixin.qq.com/s?__biz=MzUzNjAxODg4MQ%3D%3D&chksm=fafded91cd8a6487ff3a43390b87d7e55e75ca97b26446a4264d93a60b34a8d03378c9f1bca6&idx=1&mid=2247485247&scene=21&sn=62cbc747bea99d91b45104863640f95b#wechat_redirect
[java_Closeable_Cloneable]: http://mp.weixin.qq.com/s?__biz=MzUzNjAxODg4MQ%3D%3D&chksm=fafdec5ccd8a654ae722ea8fca134e09c8c4a709fbe1fdbe1f70bdcf107aeda76b794ac7cfac&idx=2&mid=2247485170&scene=21&sn=9209febce758cbd385a0f60f68b9eca7#wechat_redirect
[JAVA]: http://mp.weixin.qq.com/s?__biz=MzUzNjAxODg4MQ%3D%3D&chksm=fafdecaecd8a65b83f622d170c5ebd328c7b8071a935a19c768e9ebe8632db20588f0fb6d9f2&idx=1&mid=2247484928&scene=21&sn=dc3ce3b2f4c76cb712d384843e241cc8#wechat_redirect