CodeReview作用:
改善代码质量:
- 可以让团队其他同学帮忙协助把关代码质量,发现代码中潜在的质量问题,并给出改进建议,从而推动团队整体代码代码质量的提升。
能够实现知识共享,统一认知:
- CodeReview过程是知识碰撞的过程,是学习别人的知识体系促进自我成长的过程。
- 通过CR这样的过程能够将不同成长阶段的成员之间知识水位尽量拉齐,能够有效的提升团队编程的整体水平。
能够及时潜在安全和性能问题等:
- 能够及时发现代码中潜在的安全问题和性能问题。
- 例如:SQL注入问题、方案安全漏洞问题、部分业务场景查询实现性能较差等问题。
CodeReview关注哪些
关注点分类 | 关注点细分 | 核心关注点 | 常见问题 |
---|---|---|---|
代码规范与质量 | 命名 | 变量命名、方法命名、类命名、包命名 | 命名拼写错误、命名与实际含义不符(超出或小于)、用词不准 |
注释 | 是否有注释、注释是否合理 | 通篇无注释、注释不准确 | |
日志打印 | 日志打印级别、日志打印参数、日志格式、异常打印、是否应该打印日志 | 日志打印级别误用、日志参数未打印、日志格式不规范、异常信息未打印、打印日志过多 | |
异常处理 | 异常是否抛出、是否规范的异常编码等 | 异常该抛出未抛、肆意的使用RuntimeException等 | |
逻辑正确 | 业务逻辑是否正常 | 空指针问题、逻辑不正确等 | |
代码风格一致 | 代码风格与应用整体风格一致 | 代码风格不一致(如) | |
代码复杂度 | 圈复杂度 | 大量嵌套if导致非常复杂等 | |
架构设计 | 关注分层 | 分层是否合理、是否存在跨层调用 | 分层混乱、跨层调用 |
关注扩展性 | 扩展适配 | 硬编码扩展性低 | |
业务域划分 | 业务域划分清晰 | 业务域划分混乱 | |
性能问题 | 慢SQL | 索引设计、是否存在慢SQL语法 | 索引未设计、慢SQL用法如like %xxx 语句等 |
缓存设计 | 添加缓存、是否存在缓存击穿问题 | 该加而未加缓存、缓存击穿问题等 | |
安全性问题 | 是否存在安全风险 | 文件上传验权、越权访问问题 | 文件上传未验权、越权访问数据等 |
代码规范与质量
首先要关注代码的规范和质量问题,包括命名、注释是否合理、日志打印规范、异常处理是否合理等等。
命名问题:
推荐使用简洁明了带有业务语义的命名方式。
不建议使用
tmp、list、a、b
等无法表述清楚当前变量是什么含义的命名方式。不建议使用下划线命名的变量名称如:
_aop_signature
。
常量命名:
常量命名方式推荐大写字母,多个词之间通过下划线连接。
方法命名和类命名:
采用合适的词描述清楚当前方法/类的功能即可,注意采用驼峰式命名,代码可读性更高。
注释
注释恰当:
能够清楚的表达当前行/方法/类的功能,不建议出现注释与代码不匹配情况。
适当的注释:
建议对于较为复杂的代码要提供适当的注释。
都说好的代码是自注释的,但是无法保证的是有较复杂的业务背景情况下还有人能够看明白,此时需要添加部分注释。
日志打印
日志打印级别是否合理:
建议采用适当的日志级别进行打印,比如系统异常情况采用error级别打印。
如果只是打印流程中某一个结果可以选用info级别,当部分参数不符合预期的情况下可以采用warn。
日志打印关键参数:
建议在日志打印中打印出关键的参数信息,否则很难定位异常原因。
异常信息是否打印:
异常处理日志打印时建议打印异常信息(e)到日志中,便于排查问题。
日志是否打印过多:
尤其是针对服务调用频繁的服务,需要重点关注是否有必要打印日志,高QPS服务容易导致日志打印过多,引发磁盘容量不足等风险。
异常处理
经常会遇到应该抛出异常的地方未抛出导致业务流程流转,进而导致部分流程数据异常。
逻辑是否正确
要适当检查代码中是否存在逻辑性错误,比如常见的空指针问题、业务逻辑处理不正确等等。
代码风格是否一致
代码风格包含很多,包括类的目录归属、参数检查的方式等等,不同的应用可能不尽相同,尽量与之前的代码结构和方案保持一致。
代码复杂度
有一些代码没有逻辑问题也没有风格问题,但是看起来特别复杂,比如经常会看到有if嵌套过深的问题。
架构设计
比如分层是否合理,是否具备扩展性、业务域划分是否清晰等等。
关注分层
大部分应用都有自己的分层方案,增量的需求需要尽量保持一致的分层方案以使得分层架构统一,便于后期维护。
比如,不同分层对应的类的命名方式有所区分,所以综合分层来看需要放在正确的分层中或者统一正确的命名方式。
关注扩展性
部分业务场景在开发的过程中由于限定于当前的业务诉求,未对后续的扩展能力做提前的规划,这种情况可以在CR阶段指出,引导代码编写者思考如何进行扩展性设计。
关注业务域划分
业务域划分不清晰的系统往往更容易腐化,因为一旦业务域划分比较乱往往分不清应该放在哪个域的模块中,再加上人员迭代,会使得系统愈发的混乱。
性能问题
慢SQL问题:
在CR的过程需要关注是否有可能发生慢SQL,此过程需要结合数据库索引设计和代码中的使用场景来共同完成定位。
关注缓存设计:
部分接口在开发中并未添加缓存,如果流量较大的情况下容易导致雪崩,所以针对流量较大的场景要关注缓存设计是否合理,甚至是否有必要添加二级缓存等。
安全性
比如典型的无登录校验的上传接口、越权访问问题、SQL注入风险等等。
CodeReview文化建设
建设代码规约:
团队范围内的代码规约需要团队层面能够达成共识。
CR形式多样化:
可以针对某一份代码在固定的时间(周会等)中进行分享并带领大家一起进行Review,增强大家的参与感和趣味性。
也可以跨越领域去交换CR别的团队的代码,学习一下别的团队的代码规范。
调动积极性:
鼓励团队同学积极参与,结合奖励机制不断的引导。
坚守文化:
文化的形成,关键还在于坚持,只有坚持长期主义才能形成良好的CR文化。