昨天终于Code Review完了,提前完成任务
标签: 后端
upupor   624   4 2020-04-30 10:39 
最新一次编辑的原因:

这一周带着团队成员在进行Java代码的Code Review

中间因为太忙,断了一天,昨天下班之前,把Java代码全部Review完了,在Review过程中,收获还是蛮多的,对于代码、对于业务的熟悉度都比之前好了太多。

在没有review之前可能对于代码还没有多大的信心,review完了之后,个人对代码还是比较认可的,有信心是没有问题的

 

在review过程中,更多的关注点还是在于业务、在于逻辑,代码写的好不好这个另说,改起来也很快。业务逻辑如果不清楚,那是很致命的。所以在review过程中,如果发现有业务逻辑不清楚的,我都让团队成员去画流程图,通过这种方式来理清思路。

团队成员绘制好流程图之后,会先进行评审然后再去实施。通过这种方式解决了业务上阻塞的问题点,我能看到的是团队成员对自己的代码非常有信心,因为都是梳理过的!

 

code review我们会观察以下几个点:

  1. 函数完整性
  2. 代码空指针,经典的npl问题
  3. 代码逻辑性
  4. 业务契合度

这4个方面。

 

首先说,函数完整性。

一个函数是否完整,是否可靠,从入参校验开始。很多业务开发人员说,函数是我自己开发的,它传过来是不为空的,不用判断。对,自己写的可以保证,但是如果别人要使用你的函数呢,怎么办?如果不去校验入参,这个函数就是不健壮的,散的,有问题的。所以函数一定完整,给任何人使用应该都要保证可用,不会出现异常。保证函数完成性我们要做到,要校验的就校验,要抛异常的就抛出异常等等

 

再说代码空指针问题

我们只要坚持一个原则就是,不要相信我们轻而易举就能得到的东西。比如 user.getUserName ,获取用户的名字。首先你要判定user有没有问题,没有问题再去get获取。npl说复杂也复杂,说简单也特别简单,对于业务开发或者中间件开发者来说,仔细点,多加几层校验,就没有问题

 

代码逻辑性

代码要有基本逻辑,开发者只要思维清晰,代码一般没有问题。没有逻辑的,画流程图吧,然后跟着流程图去重构您的代码,相信我,当你做完这些事情,问题就解决了。这里推荐流程图绘制网站 processon.com ,可以点击这个链接进入 https://www.processon.com/i/5966d5b8e4b08e72e48a920e  ,这样我可以获得免费的空间(哈哈,就当做码字的奖励吧)

 

最后说一下,业务契合度

代码是否与业务契合,即在实现业务逻辑或者修改bug时,是否顺畅,如果十分顺畅那恭喜您,契合度很好,代码写的也很好。但是如果新增一个功能或者修改一个bug,会影响很多地方,那代码就有严重的问题,和业务契合度非常低,要考虑是否要重新梳理业务,来一次重构。

 

(结束)


本作品系原创,采用《署名-非商业性使用-禁止演绎4.0 国际》许可协议.转载请说明出处
本文链接:https://www.upupor.com/u/20043010391397126144 复制分享
评论4
liyanggyang

liyanggyang 2020-05-07 10:13 · 回复

我觉得还与一个挺重要的,就是圈复杂度。复杂度太高之后,时间稍微一长很容易忘记当初写代码时候逻辑,这时候再去读代码会很费脑,甚至根本不敢改(若不是有非常完备的单元测试前提下)。

对于楼主所说的“函数完整性、代码空指针问题”,我也觉得校验是很有必要的,毕竟团队开发项目不能保证每一个人都是按照规范来做。

额外引申下,我个人对函数返回值的一个见解,互相讨论下:

  1. 如果是get函数,返回单个对象,我觉得可以返回null,也可以返回空对象(空对象有个不好之处就是在于判断是否存在时,没==null这么方便)
  2. 如果是list返回一个容器类如:集合(包括map)、数组,我觉得必须返回空容器(如 new ArrayList<?>(0))
liyanggyang

liyanggyang 2020-05-07 10:15 · 回复

wps的流程图也挺好用的,登录个账号也可以共享。

upupor

upupor 2020-05-07 10:16 · 回复

@liyanggyang : 我们在code review的时候对于过于复杂的代码会进行整改,所以我们的代码一般是比较简洁容易理解的。

 

我对函数的返回值,和你是一致的,我也要求他们注意返回值和方法命名

upupor

upupor 2020-05-07 10:17 · 回复

@liyanggyang : wps 的流程图我还没有用过,我一般用 processon 和 draw.io(是免费和开源的,可以自己部署)