知乎上有个问答:大家的公司的 Code Review 都是怎么做的?遇到过哪些问题?很多答主都提到Code Review的作用是提前发现bug、提高代码质量,顺带统一团队编码规范等等。

640?wx_fmt=png&wxfrom=5&wx_lazy=1&wx_co=1


秀一下我们的几次CodeReview。


提前发现bug

当了爸妈了之后,人都难免有“不让孩子再吃自己吃过的苦”这样的想法。其实reviewer也会有这种“老父亲/老母亲”的心理:“不让别人再踩自己踩过的坑”。


比如这段讨论。为了把一个对象中的字段转为Map(key为字段名、value为字段值),我们有这样一段代码:

image.png



围绕这段代码,我们有这样一段review(聊天):

640?wx_fmt=png&wxfrom=5&wx_lazy=1&wx_co=1


首先,有同事L在review时提出可以用反射来做,并表示在另外的某个地方有现成的代码可供参考。随后,另一位同事S微微一笑:“在你来之前我们就用过反射了”,并指出了使用反射可能带来的问题。同事L恍然大悟,在同事S的明教之下,修改了他的代码,避免了一个由于对技术理解不够透彻而导致的bug。


那是不是只有技术大牛能够在review中提前发现bug呢?显然不是。除了技术不足的原因,对业务理解不够也会导致bug。而这种bug同样可以在“老父亲/老母亲”的目光下被提前发现并解决。

例如,某次需求在系统增加了这样两个枚举:

image.png


关于这两行代码,有这样一段review:

640?wx_fmt=png&wxfrom=5&wx_lazy=1&wx_co=1


这段review与技术无关,它指出的是一个纯粹的业务bug:这个枚举所处理的业务要求它的构造方法中的第一个参数必须是“xxx.SUCCESS”或者"xxx.FAIL"格式。虽然写代码的同事并不熟悉这个业务要求,但是好在熟悉业务的同事参与了review,并提前发现了这个bug。


那么,是不是只有精通技术、或者熟悉业务,才能参与code review,并发现潜在的bug呢?其实也不是。我们再看看这段代码:

  • image.png

这里的review简单明了地指出其中的问题:

640?wx_fmt=png&wxfrom=5&wx_lazy=1&wx_co=1


这个bug的原因与业务无关,也不涉及多么高精尖的技术,纯粹就是开发人员粗心大意造成的。只要认真参与review,就能发现这种粗心大意、一时手误的问题。


提高代码质量

提高代码质量、提升开发技术,应该说是每个程序员的目标。实现目标的方式有很多,参与code review就是其中一种。无论是看别人写的代码、还是看围绕代码展开的讨论,都有“他山之石可以攻玉”的作用。


例如,我们有这样一个类:

image.png


关于它,我们有这样一段review(聊天):

640?wx_fmt=png&wxfrom=5&wx_lazy=1&wx_co=1

看完这段review,是不是对SpringMVC和HttpServletRequest有多了一点了解?如果有兴趣再去钻研一下“SpringMVC其实还有其它方法来实现这个功能”,是不是又能百尺竿头更进一步了呢?


又如,围绕Controller是直接操作Response、还是封装ResponseEntity来返回InputStream,我们也有这样的review(聊天)。相信无论对三位直接参与者还是众多围观者,这段讨论都会大有裨益:

640?wx_fmt=png&wxfrom=5&wx_lazy=1&wx_co=1

640?wx_fmt=png&wxfrom=5&wx_lazy=1&wx_co=1


统一团队规范

其实个人觉得……统一团队规范这个作用在review中算是名列“后”茅的了。但是在实践中,很多review都是从检查编码规范开始的:命名啦,注释啦,换行啦,空格啦,等等。有时甚至会给人一种“review就只能检查检查代码规范”的感觉。


这种感觉当然是错觉,我们在review中引发的讨论、发现的bug就是明证。但是,就如张国荣也要苦熬十年一样,我们也在“review就只能检查代码规范”的泥潭中跋涉了很久。这其实是开展code review的必经之路。一方面,从未经历过review的团队,其代码规范肯定存在问题——要么有规范不遵守,要么压根没有规范。另一方面,对于reviewer来说,规范问题是最容易被发现一类问题。这两方面凑到一起,规范问题还不一抓一大把么?


要从这个泥潭中走出来,最根本的法子就是团队真正的建立规范、遵循规范、重视规范。只有做到了这一点,code review才能有效地提前发现bug、提高代码质量。


qrcode?scene=10000004&size=102&__biz=MzUzNzk0NjI1NQ==&mid=2247483961&idx=1&sn=712a64a46f3ddc937be22919738cc442&send_time=