〇. 概述
作者最近从别的团队接手了一个长期维护的项目, 因为项目经手的开发人员多, 代码管理不到位, 再加上开发周期紧张, 定制化需求多等问题, 导致项目内冗余代码过多, 代码质量参差不齐, 项目难以继续维护, 所以开始Review项目代码, 并重构部分代码
本文一些重构思路 及 '俏皮话' 来自 重构（第2版）[1]
设计模式参考 JavaScript设计模式与开发实践[2]
ps: 为防止代码泄露, 文章展示的代码已做脱敏处理, 非项目原始代码
一. 糟糕的命名
当我们读侦探小说时, 通过一些神秘文字猜测故事情节是一种很棒的体验, 但如果是在阅读代码, 这样的体验就不怎么好了
1. 拼写错误
<!-- 展示全部 -->
<divv-show="showAllSwith">...</div>
<!-- 展示部分 -->
<divv-show="!showAllSwith">...</div>我猜你想写的是showAllSwitch, 但是我觉得叫isShowAll是不更好一点, 因为这是一个状态, 不是一个动作
try {
...
} catch (error) {
console.wran('xxx异常',error)
}应该是console.warn 这个拼写错误就比较恶劣了, 代码运行到这会报错
const isShow = flase你说的是false ? 这个也很恶劣
ps: 如果你也经常遇到拼写错误的问题, 可以装一个拼写检查插件, 或许会有所帮助
2. 命名不规范
<divclass="project-list simple_style">
<pclass="userName"></p>
<buttonclass="DelBtn"></button>
</div>Css相关的命名, 比如样式类名, 关键帧名, 动画名等建议统一用短横线命名法, user-name, simple-style这样
HTML规范本身对标签名、属性名和属性值的大小写不敏感 ，但是W3C官方建议统一使用小写字母
const getsomelist = (ProjectKey) => {
...
}js的变量/函数名建议统一驼峰命名法
classsomeClass{
...
}类名的首字母应该大写
良好的命名规范能够使代码的可读性更高, 也方便与其他人共同开发. 用清晰明确的命名可以帮助其他人更快的理解代码的意图和功能, 也更易于修改
可以借助eslint, husky等工程化工具规范成员的代码
二. 重复的代码
一旦有重复代码存在, 阅读这些重复代码时你就必须加倍仔细, 留意其间细微的差异, 如果要修改重复代码, 你必须找出所有的副本来修改
如图, 这个用来格式化时间的工具函数被定义了三次, 如果不凑巧, 在你需要修改这个函数的时候不知道它被定义了3次, 仅更改了其中一个函数, 你以为你更改了代码的行为, 但是那些引用其他位置函数的代码还是原来的逻辑, bug就这样被制造了出来...
除了重复的工具函数, 这个项目的问题还有多次封装了axios, 重复的业务逻辑代码...
三. 不合适的Vue代码
1. v-for 和 v-if不应该写到一个标签上
<ul>
<li
v-for="user in users"
v-if="user.isActive"
:key="user.id"
>
{{ user.name }}
</li>
</ul>当v-for 和 v-if同时存在于一个节点上时，v-if 比 v-for 的优先级更高(在Vue2中v-for优先级更高)。这意味着 v-if 的条件将无法访问到 v-for 作用域内定义的变量别名
如果你想控制每个循环出来的元素, 可以在外先包装一层 <template> 再在其上使用 v-for
(在vue3中key 应该设置在 <template>上, 但在vue2中key 应该设置在子节点上)
修改建议:
<ul>
<templatev-for="user in users":key="user.id">
<liv-if="user.isActive">
{{ user.name }}
</li>
</template>
</ul>2. 计算属性computed必须有返回值
computed: {
someData() {
if (flag) {
const showName = firstName + lastName
return showName
}
}
}修改建议:
computed: {
someData() {
if (flag) {
const showName = firstName + lastName
return showName
}
return''
}
}3. 计算属性computed 的getter不应该有额外 '副作用'
const listShow = computed(() => {
let newTitle = ''
const newList = list.value.filter(item => {
if(item.title) newTitle += item.title
return item.flag
})
title.value = newTitle // 副作用
return newList
})如果你确实需要这些'副作用', 可用考虑用别的方式实现, 比如将这部分逻辑用watch实现
4. 不应该修改props， 这会破坏vue的单向数据流
props.title = 'newTitle'5. Vue保留属性key
const props = defineProps({
key: String
})key是Vue内置的用于虚拟 DOM diff 算法的特殊属性, 请不要使用key作为自定义组件属性
关于更多的Vue代码规范, 可以参考我之前写的这篇文章: 团队Vue代码规范 - 8个需要避免的错误和6条强烈推荐的规范[3]
四. 一些Js语法问题
1. 数组单纯遍历时可以用 Array.forEach / for of 等 没必要使用Array.filter, Array.map
虽然filter, map也有遍历数组的功能, 但其用途一般是根据原数组造一个新数组, 所以如果只是单纯的遍历, 用forEach就可以了, 这样语义化更好
someList.filter(item => {
item.name = someMap[item.key]
})
anotherList.map((item, index) => {
if(item.type === '01'){
updateInfo(item, index)
}
})上面两个语句只做了遍历, 没有生成新数组, 用Array.forEach就可以了
数组方法参考文档: Array 对象[4]
2. 用了Array.map, 但没完全用
const showCodes = computed(() => {
const codes = [];
sonmeList.map((item) => {
codes.push(item.code);
});
return codes;
});Array.map的作用就是根据一个数组生成一个新数组, 可以改成这样:
const showCodes = computed(() => {
return sonmeList.map((item) => item.code);
});3. 多余的三目运算
count > 0 ? (flag = true) : (flag = false)> 运算符返回的就是一个布尔值, 所以这个三目运算符没有必要, 建议改成:
flag = const > 0plan === 'A' ? (config.type = 'planA') : ''这个三目运算的第二个分支没有做任何操作, 直接用if就好了, 或者用&&, 建议改成:
if (plan === "A") config.type = "planA"
// OR
plan === "A" && config.type = "planA"4. 嵌套过深的三目运算
let code =
oldCode === "1" || oldCode === "2"
? "A"
: oldCode === "3" || oldCode === "4"
? "B"
: oldCode === "5" || oldCode === "6"
? "C"
: oldCode === "7" || oldCode === "8"
? "D"
: "Z";三目运算是可以嵌套写的, 但是当嵌套层数过多可读性和可维护性就会变得特别差, 所以建议三目运算嵌套不超过两层, 上面的例子建议改成:
let code;
switch (oldCode) {
case"1":
case"2":
code = "A";
break;
case"3":
case"4":
code = "B";
break;
case"5":
case"6":
code = "C";
break;
case"7":
case"8":
code = "D";
break;
default:
code = "Z";
}5. switch语句没有块级作用域
switch (code) {
case"0":
let text = "";
list.forEach((element) => {
text += element.text;
});
return text;
break;
}switch的分支语句没有块级作用域, 如果确实需要块级作用域, 可用使用{}将代码包起来:
case"0":
{
let text = "";
list.forEach((element) => {
text += element.text;
});
return text;
}
break;
}6. 使用import引用了模块但是没使用, 或者重复import
长期多人维护的项目特别容易发生这个问题, 可能删除了一些代码导致一些引用进来的方法已经不使用了, 但是import还留着, 或者之前引用过, 但是改的人没注意又引用了一次...
这种情况有一个很方便的解决方法: VSCode提供了一个快捷键 alt shift O可以一键整理模块引用 (ps: 这是我用过最爽的快捷键)
五. 一些样式问题
1. 多余的font-family
项目内大量指定了font-family属性, 大部分都是苹方, 经了解应该是直接从设计稿(蓝湖/摹客)提供的样式直接粘过来的(设计老师用的mac, 默认就是苹方字体), 其实大部分情况不需要指定字体, 让浏览器用默认字体渲染就可以了
一般来说, mac默认字体就是苹方, 而win电脑里不会安装苹方字体, 所以设置font-family为苹方意义不大
就算要设置项目的字体的话在全局的样式表设置一次就好了
2. 过时的float布局
有部分代码布局用的float, float布局已经过时了, 绝大部分场景用flex可以替代
3. 可以简写的属性
比如padding: 0px 30px 0px 30px; 可以简写成 padding: 0 30px;
不过这个问题不大, 后者看起来更简洁一些
4. 样式覆盖
.box{
margin-top: 20px;
margin: 050px030px;
}上面的margin-top: 20px;会被下面的margin: 0 50px 0 30px;覆盖掉, 不起作用
Css的代码风格可以使用StyleLint检查
StyleLint的使用可以参考我之前写的这篇文章: css 代码格式化工具 - Stylelint 使用指南[5]
六. 一些其他的问题
1. 混乱的全局变量
全局数据的问题在于, 从代码库的任何一个角落都可以修改它, 而且没有任何机制可以探出到底哪段代码出了问题
这个项目每个组件都可以直接操作store, 也就是说代码里任何一个地方都可能在直接更改全局变量, 如果你发现某个全局变量的值和你预期的不符, 你根本无从下手, 每个地方中都有可能更改这个变量, 也就是说, 全局变量变得不可靠
2. 根节点client-only
这个项目用了nuxt框架的ssg模式, 但是在某些页面的根节点直接使用了client-only包裹, 完全失去了ssg的意义..
问了下应该是当时遇到了水合问题, 着急修复就直接在根节点包了client-only...
3. babel相关的依赖应该放在devDependencies
babel等一些依赖只在开发环境会用到, 不应该放到生产依赖里, 这样可能会导致产物
4. 作为一个移动端的项目引用了element-plus
经排查, 是一个需要展示表格的地方使用了el-table, 而且只是展示一个简单的表格
我觉得这个功能完全没必要动用element-plus, 对于一个移动端项目来说引入element-plus太重了, 也没有必要, 完全可以自己实现(如果不熟悉html的表格标签可以让ai帮忙写一个的), 或者引用一个轻量化的表格工具
5. 生产环境引用测试资源
项目中某些图片资源直接在代码里使用了测试环境图床的链接, 且没环境判断的逻辑, 导致发布上线的项目也是引用的测试图床的资源...
测试环境稳定性相对较差, 也没有配置CDN加速, 所以如果要在项目里使用链接方式引用图片等资源, 请使用生产环境的链接
七. 一些解决思路
拼写拼写错误: 安装
Code Spell Checker插件代码语法/风格问题: 配置
Eslint,StyleLint,Prettier,husky等全局数据混乱: 使用中介模式, 代理模式等, 隔离业务代码和全局对象
其他: 定期代码
Review(可以交给AI做), 指定团队代码规范
最后
送人玫瑰，手留余香，觉得有收获的朋友可以点赞，关注一波 ，我们组建了高级前端交流群，如果您热爱技术，想一起讨论技术，交流进步，不管是面试题，工作中的问题，难点热点都可以在交流群交流，为了拿到大Offer，邀请您进群，入群就送前端精选100本电子书以及 阿里面试前端精选资料 添加 下方小助手二维码或者扫描二维码 就可以进群。让我们一起学习进步.
点个在看支持我吧
扫一扫
301
被折叠的 条评论
为什么被折叠?
到【灌水乐园】发言
