技术分享 | CodeReview主要Review什么?

573b2a591050838450f2925aa524176e.png

b8ad15390f620595a40ece68d28222bf.png

源宝导读:Code Review, 意即代码审查,是指一种有意识和系统的召集其他程序员来检查彼此的代码是否有错误的地方. 在敏捷团队中推行CodeReview, 可以帮助团队快速成长.本文将分享在"天际-建模平台"如何推行&实践CodeReview。

一、为什么要 Code Review?

要不要Code Review,需要结合当前的环境和形势来决定。如果项目组开发任务极其紧张, 此时再进行Code Review可能会收到不利的效果,因势利导,求同存异是Code Review的核心所在。

Code Review, 需要和项目组成员沟通和配合,同时也在检验各成员的技术水平。

需要注意的是,人都有惰性,每个人都有自己的一套行为准则和规范,要想把他们的行为或规范统一起来,很容易使他们产生抗拒心理。

在Code Review之前,和项目成员沟通好,有一个共同的愿景,并辅助以相应的培训,把部分人员的短板补齐。会减轻项目成员的一些不适感,也会使得项目运行更加顺畅。

同时, 也要注意,不要事事求完美主义,一步求成。"慢即是快"。


二、 Code Review的前提条件

Code Review本身属于"事后"工作,可以起到查漏被缺的作用。 

在推行Code Review时,需要先提前准备以下几个工作,以便团队能够更快更好的接受和实施。

  • 建立规范, 包含:

    • 编码规范,如变量命名,文件命名规范等。

    • 设计原则,如单一职责原则,最少知识原则等。

    • 分支管理策略。

  • 完善的文档,方便查阅。文档内容最好能共建,千万不可出现"一言堂";

  • 制定Code Review流程&目标,以及实施周期。

例如, 根据当前团队的实际情况,将Code Review实施分为3个阶段。

  • 第一阶段,重点关注,恰到好处的函数注释,"硬编码"问题,常见变量命名规则等,预期实施周期为1~3个月。

  • 第二阶段,重点关注,代码耦合性,单一职责、最少知识原则, 潜在隐患,性能问题等,预期实施周期为3~6个月。

  • 第三阶段,重点关注,模块实现方案,设计模式,最佳实践,代码重构等。

在实施过程中,如果遇到比较大的阻力或困难,推行Code Review所得到的收益较低时,可以考虑适当"休息"一段时间。


三、常见的Code Review项

3.1 git 提交规范

git commit提交规范, 如:

  • 不标注信息

  • 不及时commit

  • 提交时标注的信息不规范等

都是严重阻扰review的因素之一,除了常规的描述信息外,还可以按类型等备注:

  • feat: 新特性

  • fix: 修改问题

  • refactor: 代码重构

  • docs: 文档修改

  • chore: 其他修改

  • test: 测试用例修改

  • style: 代码格式修改等等

当然,也要以利用一些工具,来协助我们完成基本的约束和检查。

3.2 风格

3.2.1 可读性

衡量可读性,有很好的实践标准,即Code Review时能否非常容易的理解代码逻辑,如果不能,那意味着代码的可读性要进行改进。

3.2.2 命名

  • 命名对可读性非常重要,我个人倾向于函数名/类名长一点都没关系,但必须能清晰的表明函数/类的作用。

  • 英语用词尽量准确, 哪怕需要借助翻译工具,也是值得的。但有一点需要注意,如果使用的单词很冷门,都没人认识, 那就不要使用。

3.2.3 函数体长度/类长度

  • 函数体太长,不好阅读,一般建议不要超过50行。

  • 类太长, 如超过1000行,那可能就要看下是否违反了"单一职责"原则。

3.2.4 参数个数

不要太多,一般不要超过5个, 超过5个,建议使用对象。

3.2.5 注释

恰到好处的注释,能够帮助我们理解函数/类的作用。

3.3 架构/设计

3.3.1 单一职责原则

这是经常被违背的原则,也是最难运用好的原则。

  • 一个类只做一类相关的事情。

  • 一个函数/方法, 最好只做一件事情。

3.3.2 行为是否统一

  1. 什么是行为统一? 例如:

  • 错误处理是否统一

  • 错误提示是否统一

  • 弹出框是否统一

  • ......

同一逻辑/行为,有没有执行同样的代码路径, 低质量的代码一个特征是,同一逻辑/行为,在不同的地方或不同的方式触发时,没有执行同样的代码路径(产生出不同的结果),或者是各处copy一份实现,导致非常难以维护.。

3.3.3 代码污染

代码有没有对其他模块强耦合。

3.3.4 重复代码

主要看有没有把公用组件,可复用的代码、函数抽取出来。

3.3.5  开放-封闭原则

简单理解是,看代码好不好扩展。

3.3.6 健壮性

  • 核心数据有没有强制校验?

  • 对业务有没有考虑完整, 逻辑是否健壮。

  • 有没有潜在的bug?

  • 有没有内存泄露?有没有循环依赖?

  • ......

3.3.7 错误处理

有没有很好的Error Handling? 如网络出错,,IO出错等。

3.3.8 面向接口编程/面向对象接口编程

主要看有没有进行合适的抽象,把一些行为抽象为接口。

3.3.9 效率/性能

  • 客户端程序对频繁的消息和较大数据等耗时操作是否处理得当。

  • 关键算法的时间复杂度是多少? 有没有潜在的性能瓶颈?

3.3.10 代码重构

新的改动是打补丁,让代码质量继续恶化,还是对代码质量提升有帮助?


四、Code Review如何实施?

结合当前建模小组团队的实际情况:

  • 新人较多,多对业务不对熟悉。

  • 前期团队人员少,但有CodeReview的文化。

  • 已有相关的编码规范的知识沉淀,但因之前迭代任务重/人力资源紧张等因素, 导致未长期推行下来。

针对以上情况,经过团队内部协商研讨,输出相应的解决机制:

  • "结对编程",每个Story尽量安排两位同学一起完成(1位主责人),主要有两个目的:

    • 老带新,在工作实践中多沟通交流,帮助新员工尽快熟悉业务,融入团队。

    • 相互审查代码,提高代码质量,同时促进新老员工沟通交流,思想上多碰撞。

  • 两级代码审查机制

    • 第一级,由"结对编程"小组的两位同学交叉审查代码,过滤掉一些简单的代码问题。

    • 第二级,由技术leader进行最终审查,确保代码审查保质保量完成。

  • 定期分享收录的典型问题

    • 代码审查过程中,发现的一些典型问题,及时收录并做好分析。

    • 定期开展"坏代码"分享会。

主要实施分三个阶段进行:

  • 第一阶段, 恰到好处的函数注释, "硬编码"问题,常见变量命名规则等,预期实施周期为1~3个月。

  • 第二阶段,代码耦合性, 单一职责、最少知识原则, 潜在隐患,性能问题等, 预期实施周期为3~6个月。

  • 第三阶段,模块实现方案,设计模式,最佳实践,代码重构等。

从目前实施过程的体会, 以及结合同学们的一些经验来看,在Code Review时建议:

  • 单次查看代码不多于500行,人的精力有限,一次审查太多的代码,收益可能不理想。

  • 单次审查建议不要超过30分钟。

  • 祖传代码怎么办?

    • 只查看git diff中, 带有"+"的代码,历史代码可以暂时忽略不看,在不断的迭代中,迟早会遇到需要修改"祖传"代码的。

    • 这也是一种"妥协"的办法,让代码审查者和开发者之间有一个平衡。

    • 对于"开发者"的心理会产生一定的影响,他/她可能需要提前做好准备,万一哪天需要修改"祖传"代码呢?

在下一节中,收录了实施过程中发现的一些典型的问题代码,大家可以一起"品尝"下。

五、 低质量代码的常见特征

5.1 案例1

  • 违反"单一职责"原则。

  • 同一逻辑/行为, 通过不同的方式触发时, 不会执行同样的代码路径。

  • 缺少注释。

  • 函数/类命名乱, 词不达意, 或"挂羊头卖狗肉"。

export function setDefaultImg(res) {
let imgUrl = getImagePrefixUrl()
let previewImgSrc = require('@/asserts/images/icon.png')
if(res.imgId) {previewImgSrc = `${imgUrl}${res.imgId}.${res.type}?t=${+newDate()}`}
return previewImgSrc
}

上例函数,其实是想对传入的参数进行判断,如果值无效,取默认图片,否则返回拼接地址好的img src相对地址。从函数的实现来看, 有如下几个问题:

  • 函数名setDefaultImg并不合适,无任何对数据的更新/修改,最终目的是想拿到img的拼接地址(或者默认图片)。

  • 形参`res`是一个对象, 实际使用到的属性只有两个基本类型,遵循最少知识原则,应修改形参。

  • `let imgUrl`,其实是拼接url的前缀,取名如 `imgPrefixUrl`会比较合适。

  • let 使用不当,如 `let imgUrl`,因涉及不到变量再赋值,使用`const是比较合适的`。

  • `res.imgId`,即异常情况的处理,少了对`res.type`的处理。

  • 变量命名,如`previewImgSrc`,取此名也不太合适,该src使用的目的并不确定,建议使用有比较通用涵义的名字。

可考虑重构为:

export function getImgSrc(imgId, imgType) {    if(typeof imgId !== 'string' || typeof imgType !== 'string') {        return require('@/asserts/images/icon.png')    }    return `${getImagePrefixUrl()}${imgId}.${imgType}?t=${+new Date()}`}

5.2 案例2

  • 参数传递自由度过大,易引发后续不可控的风险。

handleSelect(){this.$emit('select', this.$props)
}

上例代码中,传递给`$emit`的数据是`this.$props`,会有以下问题:

  • 存在高耦合关系( 数据结构 耦合)。

接收参数,建议尽量使用:基础数据类型,如:字符串, 数字,布尔值等,非必要情况下, 不使用复杂数据类型,如: Array, Object等

可考虑重构为: 

handleSelect(){  this.$emit('select', this.$props.userId)}

5.3 案例3

  • 违反"单一职责"原则,一个函数处理多个任务,可能会在调函数时产生“异外的感觉”。

  • 形参设计过度(形参无须解构,后续若有需要添加新的参数时再添加,也可考虑使用options参数等来实现扩展)。

handleRowDelete({ row }) {      this.$refs.grid.remove(row)      let open = false      for (let i = 0, l = this.$refs.grid.innerData.length; i < l; i++) {        let item = this.$refs.grid.innerData[i]        if (item.isQueryField) {          open = true          break        }      }      if (!open) {        //文本行        this.$refs.grid.innerData[0].isQueryField = true        this.handleChange({          name: 'isQueryField',          row: { $index: 0 },          data: this.$refs.grid.innerData        })      }    }

从上面的代码中,我们可考虑从以下几个角度着手重构, 以增强可读性。

  • 在入口函数处理,调用功能函数(通过函数名来区分功能),明示调用逻辑。

  • 减少代码量,增强可读性,能用ES6 方法解决的就用它解决,尽量不造不必要的轮子,比如使用Array.findIndex来解决查找数组是否存在某一项的问题。

  • 优化业务逻辑,删除不必要的代码,增强代码的可维护性。

  • 函数重新命名,务必做到“见名知意”。

  • 添加必要的注释(遵循jsdocs注释规范)。

可考虑重构为:

mounted(){const {row} = this.getSelectedRow();this.deleteRow(row);this.resetGrid();},/*** @description: 删除指定的表格行数据* @param {Object} row, 表格行数据* @return*/deleteRow(row) {this.$refs.grid.remove(row)}/*** @description: 重置表格**/resetGrid(){if(this.$refs.grid.innerData.length && this.$refs.grid.innerData.findIndex(item => item.isQueryField) < -1){this.$refs.grid.innerData[0].isQueryField = truethis.handleChange({name: 'isQueryField',row: { $index: 0 },data: this.$refs.grid.innerData})}}

六、总结

敏捷团队由"创建-振荡-成熟-规范"的过程中, 不同的阶段建议采取不同的方式开展CodeReview, 需要考虑团队的整体情况以及当前公司的业务的紧急程度, 优先解决首要问题,再接着解决其他次要问题。

通过几个月的渐进 & 持续的推行Code Review后,团队成员是可以达到以下效果的:

  • 促进团队成员的相互交流, 加速团队朝"成熟-规范"阶段前进。

  • 知识分享, 促进团队成员进行反思和总结。

  • 代码质量和可维护性, 可读性等的提高。

  • 查漏补缺, 发现一些潜在的问题点等。

  • 最佳实践, 能够更好更快的完成任务的方法。

------ END ------

作者简介

张同学: 开发工程师,目前负责天际-建模平台产品的研发工作。

也许您还想看:

技术分享|Java SDK动态数据源和上下文机制

技术分享|NodeJS分布式链路追踪实现

技术分享 | Java SDK 元数据驱动的事件通信架构

技术分享|Hangfire深度实践

技术分享 | APT结合JavaPoet生成模板化Java源代码文件

技术分享 | 玩转高效UI自动化测试

更多明源云·天际开放平台场景案例与开发小知识,可以关注明源云天际开发者社区公众号:

【建模】文档服务提供高保真打印模式

明源云·天际硬核技术认可:获华为鲲鹏技术认证书

天际·开发者社区“重装发布”!

ea9125cc992fa45f62c8f2a7057f9616.png

本文来自互联网用户投稿,该文观点仅代表作者本人,不代表本站立场。本站仅提供信息存储空间服务,不拥有所有权,不承担相关法律责任。如若转载,请注明出处:http://www.mzph.cn/news/293472.shtml

如若内容造成侵权/违法违规/事实不符,请联系多彩编程网进行投诉反馈email:809451989@qq.com,一经查实,立即删除!

相关文章

Oracle中的USEREVN()

1.USEREVN() 返回当前用户环境的信息,opt可以是: ENTRYID,SESSIONID,TERMINAL,ISDBA,LABLE,LANGUAGE,CLIENT_INFO,LANG,VSIZE 1.ISDBA 查看当前用户是否是DBA如果是则返回true SQL> select userenv(isdba) from dual; USEREN ------ FALSE 2.SESSION 返回会话标志 SQL>…

你尿尿的时长是不是21秒?2次登上Nature封面的他,靠研究拉尿获得“诺贝尔奖”.........

全世界只有3.14 % 的人关注了爆炸吧知识男人的快乐就是这么朴实无华为什么蚊子不会被雨滴砸死&#xff1f;这个发在知乎上会被质疑患有十年脑血栓的问题&#xff0c;获得了2015年的中国搞笑诺贝尔奖——菠萝科学奖。获奖者结束获奖感言的方式也很搞笑&#xff1a;这个直接在“诺…

SQL优化总结

SQL 的优化主要涉及几个方面&#xff1a; &#xff08;1&#xff09; 相关的统计信息缺失或者不准确 &#xff08;2&#xff09; 索引问题 &#xff08;3&#xff09; SQL 的本身的效率问题&#xff0c;比如使用绑定变量&#xff0c;批量DML 采用bulk等&#xff0c;这…

php curl用法

2019独角兽企业重金招聘Python工程师标准>>> curl 是使用URL语法的传送文件工具&#xff0c;支持FTP、FTPS、HTTP HTPPS SCP SFTP TFTP TELNET DICT FILE和LDAP。curl 支持SSL证书、HTTP POST、HTTP PUT 、FTP 上传&#xff0c;kerberos、基于HTT格式的上传、代理、…

Ubuntu之12.04常用快捷键——记住这些你就是高手啦!

桌面 ALT F1: 聚焦到桌面左侧任务导航栏&#xff0c;可按上下键导航。 ALT F2: 运行命令 ALT F4: 关闭窗口 ALT TAB: 切换程序窗口 ALT 空格: 打开窗口菜单 PRINT: 桌面截图 SUPER: 打开Dash面板&#xff0c;可搜索或浏览项目&#xff0c;默认有个搜索框&#xff0c;按“…

Zabbix RCE with API JSON-RPC

测试脚本: #!/usr/bin/env python # -*- coding: utf-8 -*- # Software Link: http://www.zabbix.com/download.php # Version: 2.2 - 3.0.3import requests import json import sysdef verify(url,hostid):url url /api_jsonrpc.php ### Dont editlogin Admin ##…

类的进阶

apply的两个参数分别是上下文和参数组成的数组。 function.apply(this, [1, 2, 3]); call的两个参数是多个&#xff0c;也就是不用数组包裹参数。 function.call(this, 1, 2, 3); 常常会遇到事件内部没有this的情况&#xff0c;怎么处理呢&#xff1f; 低级方法&#xff1a; $(…

探索 dotnet core 为何在 Windows7 系统需要补丁的原因

在一些 Windows 7 系统上&#xff0c;根据 dotnet 官方文档&#xff0c;需要安装上 KB2533623 补丁&#xff0c;才能运行 dotnet core 或 .NET 5 等应用。尽管非所有的设备都需要安装此&#xff0c;但这也让应用的分发不便&#xff0c;安装包上都需要带上补丁给用户安装。此补丁…

oracle 性能优化--索引总结

索引是建立在表的一列或多个列上的辅助对象&#xff0c;目的是加快访问表中的数据&#xff1b; Oracle存储索引的数据结构是B*树&#xff0c;位图索引也是如此&#xff0c;只不过是叶子节点不同B*数索引&#xff1b; 索引由根节点、分支节点和叶子节点组成&#xff0c;上级索引…

Android之在ubuntu过滤日志以及ps总结

第一步:得到Pid 如果我们不知道TAG的情况下,先得到进程的PID adb shell ps | grep call 会显示出进程关于call的出来 比如得到pid 是1123 第二步:过滤Pid adb logcat | grep 1123 就可以看到过滤的日志了 常见的命令 杀死进程 adb shell kill pid adb shell am force-s…

Centos6.3下DRBD+HeartBeat+NFS配置笔记

--------------闲 扯------------------ 这里首先感谢酒哥的构建高可用的Linux服务器的这本书&#xff0c;看了这本书上并参考里面的配置让自己对DRBDHeartBeatNFS思路清晰了许多。drbd简单来说就是一个网络raid-1,一般有2到多个node节点&#xff0c;各个节点创建的磁盘块会…

解决centos ping不通外网

先确认三件事&#xff1a; 一。ip 二。网关 三。dns 一就不说了&#xff0c;设置好本地ip和掩码就行了&#xff0c;二网关 添加默认网关&#xff0c;命令&#xff1a;route add defaule gw 192.168.1.1 这是 你用route命令查看最下面会有一条默认路由&#xff0c;走192.168.1…

如何在单个测试中同时执行多个断言

前言虽然&#xff0c;推荐做法是每次测试只断言一件事&#xff0c;但是&#xff0c;在实际工作中&#xff0c;我们可能需要对同一个对象同时执行多个断言。例如&#xff0c;微软官方示例项目eShopOnContainers有一个测试用例的实现代码如下&#xff1a;[Fact] public async Tas…

jquery如何获取元素的滚动高度

获取浏览器显示区域&#xff08;可视区域&#xff09;的高度 &#xff1a; $(window).height(); 获取浏览器显示区域&#xff08;可视区域&#xff09;的宽度 &#xff1a; $(window).width(); 获取页面的文档高度 $(document).height(); 获取页面的文档宽度 &a…

TCP/IP模型的各层的作用

第一种总结&#xff1a; TCP/IP模型&#xff1a;以无缝方式实现各种网络之间互连的网络体系结构。 TCP/IP模型共分四层&#xff0c;分别为应用层、传输层、互联网层和主机到网络层。 各层实现特定的功能&#xff0c;提供特定的服务和访问接口&#xff0c;并具有相对的独立性。 …

JAVA解决山脉数组的峰顶索引问题

JAVA解决山脉数组的峰顶索引问题 01 题目 符合下列属性的数组 arr 称为 山峰数组&#xff08;山脉数组&#xff09; &#xff1a; arr.length > 3 存在 i&#xff08;0 < i < arr.length - 1&#xff09;使得&#xff1a;arr[0] < arr[1] < ... arr[i-1] <…

php设计模式之单例(多例),注册器,观察者模式

单例(Singleton)模式和不常见的多例(Multiton)模式控制着应用程序中类的数量。如模式名称&#xff0c;单例只能实例化一次&#xff0c;只有一个对象&#xff0c;多例模式可以多次实例化。 基于Singleton的特性&#xff0c;我们经常用Singleton配置应用程序并定义应用程序中可能…

Oracle小知识总结

1. 每天的8&#xff1a;00到23&#xff1a;00每隔5分钟执行一个sql语句的JOB --建立一个存储过程 CREATE OR REPLACE PROCEDURE p_jobtest IS v_hh VARCHAR2(2); BEGIN v_hh : to_char(SYSDATE, hh24); IF v_hh > 08 AND v_hh < 22 THEN --你的sql语句 NULL; END IF; EN…