代码评审于技术团队的工程师文化建设非常有意义，它是形成团队统一代码风格最有效的方式，作者把自己团队在一年的CR中常见的那些小问题做了一些梳理，希望能对大家起到一点小帮助。

一、前言

团队开展线下的周代码评审已有一年有余，作为主要的评委原本以为给出代码建议是需要花很多时间和心力的事儿，总也会想着能给被评审者以巨大的帮助改善代码的设计问题，实际review下来发现大部分同学还是在一些基础的代码质量问题上，改善设计或者寻找业务bug这种目标达成的评审很少出现，因而当下我认为团队对CR也还是要有足够耐心，将基础问题当做一个个bug来解说透理解透，在走向卓越工程的路上积累。与此同时也能很深刻的感觉到每个同学对待代码的态度是不一样的，这就产生了两个分化有人代码改善很快有人代码也不断的重复的出现评审过的问题，用心的代码是一定能看到设计的痕迹，用心的同学也一定会吸取一次次的小问题当做自己的成长点，借此年终大总结要上分的阶段，我把我们团队在一年的CR中常见的那些小问题做了一些梳理，希望能对大家起到一点小帮助。

二、翻车现场（CR中的常见问题）

2.1 代码规范类

2.1.1 使用魔法值
翻车指数：★★★★★说明：魔法值表示在代码中未声明而被直接使用的值，这在我们的代码评审问题中广泛存在。





危害

代码不易读

代码不被复用

容易造成代码缺陷

建议

使用声明后的常量替代魔法值！


bad case
const now = Date.now();

const lastVisitTime = localStorage.getItem('last_visit_time');



if (lastVisitTime && parseInt(lastVisitTime, 10) > 24 * 60 * 60 * 1000) {

 console.log('一天前来过');

}



localStorage.setItem('last_visit_time', now.toString());

good case
const LAST_VISIT_TIME_CACHE_KEY = 'last_visit_time';

const DAY_IN_MS = 24 * 60 * 60 * 1000;



const now = Date.now();

const lastVisitTime = localStorage.getItem(LAST_VISIT_TIME_CACHE_KEY);



if (lastVisitTime && parseInt(lastVisitTime, 10) > DAY_IN_MS) {

 console.log('一天前来过');

}



localStorage.setItem(LAST_VISIT_TIME_CACHE_KEY, now.toString());

2.1.2 滥用eslint-disable
翻车指数：★★★说明：eslint-disable 是用于在 ESLint 中禁用特定规则的指令，在遇到lint报错不好解决时就有同学会选择使用它规避问题。
现场




危害

代码质量下降：禁用规则可能会引入不推荐的代码模式，增加潜在的bug和维护难度

教育价值丢失：eslint也是一个学习工具，它可以帮助我们学习更好的编码习惯。禁用规则可能会错失学习和改进代码的机会。

建议

不是万不得已（通常是维护祖传代码场景），不要禁用lint检查。


2.1.3 使用幽灵依赖
翻车指数：★★★说明：幽灵依赖（也称为隐性依赖或隐式依赖）是指在项目的某个模块中使用了未在该模块的package.json文件中直接声明的依赖。
案例




危害

不可靠的构建，可能依赖不会被安装，依赖缺失无法完成打包；

版本问题，幽灵依赖被隐式的升级或更改，项目出现突然中断或错误；

让安全风险和依赖地狱的问题难以被追踪；

建议

使用Lint工具，启用import/no-extraneous-dependencies规则来检测未声明的依赖


2.1.4 未遵循no-else-return原则
翻车指数：★★说明：no-else-return是一个代码质量规则，它强调了代码简洁性和可读性，用于指出当一个if块中已经包含了一个return语句后，就没有必要再使用else块。因为一旦执行了if块中的return语句，代码就会退出当前函数，所以else是多余的。❌ You can skip the else block
function hello(condition: boolean) {

 if (condition) {

   return '👋';

 } else {

   return '👻';

 }

}

✅ Yay, much easier to read
function hello(condition: boolean) {

 if (condition) {

   return '👋';

 }



 return '👻';

}

2.1.5 随意的commit message
翻车指数：★★★★说明：大部分时候大家我们已经会去注意commit message的格式了，比如如何表达新功能、修复、重构等信息，主要问题是描述含糊不清太过简化，无法理解变更的上下文。
现场



危害

困难的代码评审：reviewer难以理解每次提交的目的，影响评审效率

低效的问题追踪：难以追踪bug来源，影响修复进度

项目历史混乱

自动化流程受阻：工具可能无法正确解析不清晰的commit message，比如生成changelog

建议

规范团队的commit message格式规约并用lint工具做一定的保证，可以页内著名的Angular团队的“Conventional Commits”

保证信息主题的清晰、简洁，能让其他成员快速理解提交的意图和内容。


2.2 代码风格类

2.2.1 模块引入顺序混乱
翻车指数：★★★★★说明：在JavaScript模块中，import 顺序随心所欲没有遵循一些最佳实践原则也在我们早期的代码评审中经常出现。
现场




危害

代码不整洁可读性差，可能导致重复导入同一个模块

css文件放在上方在想覆盖外部模块样式时会不生效

建议

第三方库优先

项目内部模块其次，同时先绝对路径再相对路径

样式文件最后

最好直接使用一些工具进行import排序，比如prettier-plugin-sort-imports


import 案例
// 第三方库

import React from 'react';

import PropTypes from 'prop-types';

import { connect } from 'react-redux';



// 项目内的模块

import { myUtilityFunction } from '@/utils/myUtilityFunction';

import { MY_CONSTANT } from '@/constants/index';

import Dashboard from '@/components/Dashboard';



// 相对路径

import Header from '/components/Header';



// 样式

import './style.less';

2.2.2 未使用解构（destructuring）
翻车指数：★★★
案例




建议

解构允许你用一行代码从对象或数组中提取多个属性或元素，这比逐一赋值更简洁，可以配置lint规则prefer-destructuring鼓励使用数组和对象的解构而不是通过成员表达式访问数据。


2.3 命名问题

程序员世纪大难题之一，随意命名变量、函数、类和其他代码实体会带来一系列问题，对开发效率、代码质量、团队合作和软件维护性产生负面影响。结合线下评审发现这方面重视的同学无比重视挖空心思想取些很专业很上道的名称，也有同学认为在命名问题上纠结大可不必，毕竟一个名称不会影响代码跑出来的结果。然后混乱错误的命名必定将你的代码带向深渊，因为在代码的每一个角落不好的命名可能都会误导着下一个维护者，让维护者在不断的爆粗当中堆积屎山。大部分时候我们很难正确地命名，缺乏意愿、缺乏思考、缺乏技巧都是罪魁祸首。以下列举我们常见的命名问题：

2.3.1 表意错误或者不明确
翻车指数：★★★★★说明：这是评审时出现频次最高的问题，以至于后面几个坚持要严谨对待命名问题的同学评论的都多少有些乏力。
案例




建议

如果命名不能表达出变量、函数或类的含义，简短的命名将没有任何意义


2.3.2 大量的拼写错误
翻车指数：★★★★说明：这是最不能容忍的，命个好名还讲究方法和积累的话，命名的拼写错误则是比较容易达成的事情。
案例




建议

无他，vscode插件（Code Spell Checker）可以告诉你哪里错了，重点是你得去改！


2.3.3 命名不符合规范
翻车指数：★★★说明：命名规范是我们在代码评审的过程中慢慢完善的，这比较有效地统一了大家的命名风格。
现场


建议

有规范本身比规范是不是最佳要重要得多，统一大家的命名风格是成熟前端团队值得去做的事