代码评审又称为(Code Review,简称CR),关于CR,开发同学其实都不陌生,现在大部分公司的项目开发流程中,它都是必不可少的一个环节,CR的好处也都耳熟能详。不同的公司对于CR的方式、质量要求标准都不一样。这篇文章主要讲的是在代码评审的过程中,会对哪些代码和方向进行评审,给大家在接下来的CR中作为参考。
很多项目一般把CR的时间节点放在上线前,如果项目越大,CR的节点越靠前越好,越靠后临近上线前,开发同学越不愿意去修改,因为测试过的代码只要出现改动,就有发生问题的风险,测试也需要重新回归,有可能会产生新的缺陷,或者将问题带到生产。下面是我们项目流程中CR的节点,一般会在联调前、提测后、上线前各进行一次代码评审。
1、设计方案
设计方案一般是开发前期定好的方案,在拥有一份完美的设计方案的理想状态下,在CR阶段只需在阅读代码时判断,是否遵循设计方案进行开发即可。
一份相对完整设计方案通常会包括:
但是现实情况是,现在很多公司没有设计评审环节,项目开发前并没有进行方案设计。可能有以下几个原因:
第一:项目太小,认为没有必要。
第二:没有意识到设计方案的好处,觉得浪费时间,在方案设计的过程中,需要调研、设计、画图、出方案,少则一两天,多则三四天。在紧张的开发周期内,项目都希望越早开发完成越早上线。
第三:即使开发同学做了设计方案,也不会进行设计方案的评审。因为每次项目都需要将相关人员和评审负责人拉到一起,进行半个小时或者一个小时的方案评估。所以因为各种原因,最终设计方案都成为了需求文档的变种。
一般注释是什么时候写?
经常听到这样的回答:注释应该写于代码之前,提前捋清楚逻辑和思路,将注释写好,这样也无需反复修改,代码能更加整洁清晰。其实设计方案也像是一个超级大的注释,提前整理好开发的思路,这样也能够事半功倍。但是提前做好技术设计方案不仅仅是为了在开发的过程中更加顺利,同时也是代码评审过程中的一大助攻。
很多时候我们拿到需要评审的代码都是通过文件的顺序从上而下进行逐行阅读,这种方式往往每个文件的上下文都无法衔接,理解了第一个文件但是切换到第二个文件依然不知所云,往往需要借助注释、口述或自行查阅上下文查看代码等方式进行理解。但是通过一份完整的设计方案我们便可以了解大致的需求、开发者的设计思路,并且跟随者设计方案的结构和思路对代码的主线进行阅读。
在写了设计方案并设计方案已通过评审的项目中,在代码设计方面更多的侧重主要在于代码是否遵循设计方案进行开发,设计方案的合理性、可行性、可扩展性等在设计方案评审阶段已经通过了讨论有了结论。而对于没有设计方案或没有进行设计方案评审的项目,在理解代码的同时,也需要对整体方案的设计进行评估,而,即使方案设计并不合理,一旦经过测试面临即将上线,也没有时间和条件去进行调整。
在设计上,一般会关注下面几个方面:
2、可维护性
可维护性这个词其实意味着很多,比如,符合团队规范,复杂度善可、可读性较高、可扩展性也还不错、结构合理,前面做的很多优化设计其实都在为之后的可维护做铺垫。
每个团队都有自己的一套编码规范,在评审的过程中需要注意是否符合团队的编码规范。但是大部分的编码规范都可以用工具进行约束,能够使用工具约束的内容,尽可能不必在评审时花时间去关注。可以将关注点放在工具约束之外的规范上,比如:
3、健壮性
代码是否具备安全性和健壮性,对任何一个团队来说,无疑都是非常重要的。
4、功能范围
很多人认为功能特性的范围是测试应该去保障的,代码评审时不需要去关注开发了什么功能。但其实现状是,我们上线的代码往往有很多属于夹带私货,比如,上个迭代有一个影响不大的小Bug,趁着还没被发现,偷偷将它带上线,或者,发现上一次写的代码太蠢了,还有更好的解决思路,于是洁癖发作,默默地改了,还觉得自己棒呆了。但是测试只知道本次迭代的功能特性,除了回归主功能之外,并不知道还有其他需要重新测试的地方。如果开发同学刚好对自己的代码非常自信,觉得一定没问题,没有通知到测试回归。根据墨菲定律,这种往往觉得没有问题的代码,最后...都能够引发线上故障。
5、监控/埋点覆盖
添加监控的前提是公司有一套监控系统,除了定义好的异常监控场景以外。通常新增的一些关键功能、页面等也需要加入监控,提前加好监控代码,无需等到要查问题时,才记起来忘记加监控了。
6、合规
合规这个词在金融行业非常普遍,但也因为随着人们越来越注重隐私和安全,法律法规日渐完善,对合规这个词也不再陌生。如果应用不合规,就将面临被下线的风险,而有一部分不合规的内容,可能无法通过测试测出。所以在CR的过程中对合规性问题的审查,就尤为重要。任何使得用户隐私泄露的操作都需要禁止。
7、性能
C端应用对前端的性能要求会比较高,代码评审时也可以关注一下比较常见的问题。前端性能优化 24 条建议(2020)
8、tips
前段时间听了我司一位资深CR大佬的讲座,列了很多平时在CR过程中发生的一些常见问题,我在此基础上进行了一些新增和扩展,供大家参考。
第三方库
第三方库的新增、删除、版本号的修改(包括新增小箭头),都要确认好修改范围,确保了解库升级所带来的影响。不得随意修改版本号,第三方库哪怕是小版本的升级也不能保证对当前项目或者当前依赖包没有影响,为了避免造成线上问题,最好锁死版本号。
// package.json
// before
{
"dependencies": {
"eslint": "7.27.0",
"eslint-plugin-vue": "7.15.1",
},
}
// after
{
"dependencies": {
"eslint-plugin-vue": "^7.15.1",
"typescript": "^4.3.2"
},
}
命名
命名不统一,导致阅读的成本过高,同表示数量,但是在a文件命名quantity,b文件命名amount,c文件命名count。
// a.js
const quantity = 1;
// b.js
const amount = 1;
// c.js
const count = 1;
循环引用
JavaScript 模块的循环加载
文件的相互引用,可能会导致引用报错undefined。尽量避免文件之间的相互依赖,可以使用eslint或者webpack进行约束。
// a.js
import b from b.js'
// b.js
import a from 'a.js'
复杂的判断条件
一般逻辑判断非常复杂一般前期没有想得很清楚,或者后期的维护不断的迭代,持续往上叠加,在这种情况下,逻辑会变得越来越复杂,在开发或CR时可以考虑重新梳理清楚,重点查看,进行优化。
if (!somethingA || somethingB && (!somethingC || somethingD)) {
...
}
or
if (...) {
if (...) {
...
} else if (...) {
...
} else {
...
}
} else {
...
}
逻辑范围变化
此问题一般出现在增量代码上,因为前面的条件判断的范围变小了,导致后面的逻辑处理的范围变大了。此时要注意这种范围的变化,是否真的符合预期,还是只想修改一处逻辑,却不小心影响到了后面的逻辑。
// before
if (type === 1) {} else {}
// after
if (type === 1 && isShow) {} else {}
异常处理
确保所有的边界逻辑都已经处理或者无需处理。
// else为空,确认无需处理
if (conditionA) {}
// 报错catch
UserService.getList().then()
// try...catch,未处理catch
try {...} catch() {}
⼤概率正确 !== 正确
前面说过的墨菲定律(墨菲定律真好用):凡事只要有可能出错,那就一定会出错。下面setTimeout的取值方式相信很多人都见过,使用500毫秒的延迟,使偶现的问题,“不再出现”。但其实只是将出现的概率变小了,该出现的问题还是会出现。治标不治本。一定要找到出现问题的原因,真正解决它。
setTimeout(() => {
...延时拉取接口 or do something
}, 500)
=> 正确 !== 小概率出错
object链式取值
监控平台总是会出现很多这样的报错xxx is undefined,大部分的原因主要是因为我们在对象取值时,喜欢直接点点点。建议使用lodash的get或者?.??。
const a = productObj.something.a;
=> productObj.something是否一定有值?
// lodash
const a = get(productObj, 'something.a')
// 或者双问号操作符
?.??
隐式类型转换
隐式类型转换容易出现很多问题,==可以使用eslint避免。
if ( amount == '22' ) {}
+new Date()
css重复样式
很多时候CR时都会忽略css,觉得它翻不出什么浪花,但是在像小程序这种对代码包的体积有严格要求的项目中,CSS的精简就显得非常重要了。很多时候在写样式时,都会根据视觉稿一股脑全部写完,但其实很多可以继承的属性,是不需要重复书写的。
.page {
font-size: 18px;
font-family: sans-serif;
line-height: 18px;
color: #000;
...
.box {
font-size: 18px;
font-family: sans-serif;
line-height: 18px;
color: #000;
...
}
}
每个团队对CR的要求和规范都不一样,大家可以选择适合自己的。
原文来自:https://segmentfault.com/a/1190000040780312
一个系统可以维持5年,10年,甚至20年以上,但是代码和设计模式的生命周期非常短,当对一个解决方案使用不同的方法进行迭代的时候,通常只能维持数月,数日,甚至几分钟的时间
良好的编程习惯涉及到很多方面,但在软件行业内,大多数的公司或组织都不会把良好的编程习惯列为主要关注点。 例如,具有可读性和可维护性的代码比编写好的测试代码或使用正确的工具更有意义,前者的意义在于可以让代码更易于理解和修改。
减少嵌套会让代码可读性更好,同时也能更容易的找出bug,开发人员可以更快的迭代,程序也会越来越稳定。简化代码,让编程更轻松!
Google为了那些还不熟悉代码规范的人发布了一个JS代码规范。其中列出了编写简洁易懂的代码所应该做的最佳实践。代码规范并不是一种编写正确JavaScript代码的规则,而是为了保持源代码编写模式一致的一种选择。
程序员似乎忘记了软件的真正目的,那就是解决现实问题。您编写的代码的目的是为了创造价值并使现有世界变得更美好,而不是满足您对自我世界应该是什么的以自我为中心的观点。有人说:如果你拥有的只是一把锤子,那么一切看起来都像钉子一样
TinyMCE是一个轻量级的基于浏览器的所见即所得编辑器,由JavaScript写成。它对IE6+和Firefox1.5+都有着非常良好的支持。功能方强大,并且功能配置灵活简单。另一特点是加载速度非常快的。
函数式编程对应的是命令式编程, 函数式编程的核心当然是对函数的运用. 而高阶函数(Higher-order)是实现函数式编程的基本要素。高阶函数可以将其他函数作为参数或者返回结果。所以JS天生就支持函数式编程
朋友发表了一条说说:入职新公司,从重构代码到放弃”,我就问他怎么了?他说,刚进一家新公司,接手代码太烂,领导让我先熟悉业务逻辑,然后去修复之前项目中遗留的bug,实在不行就重构
页面实现关键词高亮显示:在项目期间遇到一个需求,就是搜索关键词时需要高亮显示,主要通过正则匹配来实现页面关键词高亮显示。在搜索结果中高亮显示关键词:有一组关键词数组,在数组中筛选出符合关键字的内容并将关键字高亮
软件工程学什么? 学计算机,写程序,做软件,当程序员。听说学计算机很辛苦? 是的,IT行业加班现象严重。在计算机世界里,技术日新月异,自学能力是程序员最重要的能力之一。选了这个专业,就要时刻保持好奇心和技术嗅觉,不能只满足于完成课内作业。
内容以共享、参考、研究为目的,不存在任何商业目的。其版权属原作者所有,如有侵权或违规,请与小编联系!情况属实本人将予以删除!