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

我也来秀一下我们的几次CodeReview。

提前发现bug

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

技术问题

比如这段讨论。为了把一个对象中的字段转为Map（key为字段名、value为字段值），我们有这样一段代码：

private Map<String, String> buildBankCreditMap(Bank bankCredit) {
        Map<String, String> map = new HashMap<>(70,1L);
        map.put("productCode", bankCredit.getProductCode());
        map.put("name", bankCredit.getName());
        map.put("idNo", bankCredit.getIdNo());
        map.put("mobile", bankCredit.getMobile());
        map.put("cardNo", bankCredit.getCardNo());
        map.put("education", bankCredit.getEducation());
        map.put("marriage", bankCredit.getMarriage());
       map.put("email", bankCredit.getEmail());
       // 后略
       return map;
   }

围绕这段代码，我们有这样一段review（聊天）:

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

业务问题

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

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

/**
     * 新增 75
     */
    XXX_SUCCES("XXX_SUCCES",75,75),
    /**
     * 新增 78
     */
    XXX_SIGN("XXX_SIGN",78,78),

关于这两行代码，有这样一段review：

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

粗心大意的问题

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

private List<BankCardListVO> queryBankCardListVOs(List<BankListDTO> bankListDTOs){
        List<BankCardListVO> bankCardListVOs = new ArrayList<>();
        BankCardListVO info = new BankCardListVO();
        for (BankListDTO bankListDTO : bankListDTOs){
            info.setBankId(bankListDTO.getCode());
            info.setBankLogo(bankListDTO.getIcon());
            info.setBankName(bankListDTO.getName());
            bankCardListVOs.add(info);
        }
        return bankCardListVOs;
    }

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

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

提高代码质量

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

例如，我们有这样一个类：

/**
 * 对原生的 {@link HttpServletRequestWrapper} 做一次包装，使之可以添加参数。*
 * @author ***
 * @date 2019年4月12日
 */
public class RequestParamWrapper extends HttpServletRequestWrapper {
    // 略
}

关于它，我们有这样一段review（聊天）：

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

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

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

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

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