1.关于CR责任划分的一点感悟
code-review
又名 CR
, 通常CR只会review代码规范,代码逻辑,而不会关心业务功能。
有人说线上出事故,做CR的人也会一定的责任,我觉得是不对的:
- 线上事故,一般都是页面报错,业务逻辑有错误,没人看的到代码,而页面或者业务逻辑有错,我觉得跟负责这次需求的相关人员有关。
- 如果说CR要承担责任,那至少要给做CR的人一定的奖金激励,不然谁愿意做这个吃力不讨好的事。
2.我为什么要求CR
自从换组后,我也是主动要求代码合并master前,需要CR 并 承担起了 CR的责任。 因为现在的代码实在是不忍直视,重构风险又太大,只能从现在做起了。
- 可以锻炼一下自己CR的能力
- 看看同事写的代码,多与同事沟通,加深联系
- CR后的代码,代表团队最高水平
- 说不定可以提前预定晋升名额,hahah 😄
3.我是如何CR的
一、规范类问题
1.1 ts项目要体现出ts的优势,通常用枚举来表示,能够让人一眼知道表达的是什么
解:
enum TABS {
TAB_1: '1',
TAB_2: '2',
TAB_3: '3',
TAB_4: '2',
}
setActiveKey(TABS.TAB_1)
1.2 主题这种,项目里很多地方都会用到,直接写字面量,以后要改岂不是很麻烦
解:
enum THEME {
LIGHT: 'light',
DARK: 'dark'
}
interface IAppStoreState {
theme: THEME.LIGHT | THEME.DARK;
}
1.3 不审核代码不知道,还真的有人这么写
解:
tab_bar_style = {
style: {
color: 'blue',
background: 'red'
}
}
1.4 代码简写
这种map结构, 不管是switch case 还是 if , 看起来都感觉有点累赘。 解:
const STATUS_MAP = {
'success': 'Successfull',
'fail': 'failed',
'warn': 'warning',
'danger': 'dangerous',
'info': 'information',
'text': 'texts'
}
return STATUS_MAP[status] ?? status
1.5 代码简洁之道
尽可能要让代码可阅读性强一点,jsx很灵活,但是一股脑堆逻辑,后面看起来就会眼花缭乱,也不好维护,我们要尽可能少嵌套,适当抽象
1.6 TDD/BDD 目录划分问题
statusMap 是某一个模块下使用的对象,而src/utils/index, 属于公共模块
项目通常来说都会按高内聚、低耦合
开发的模式
模式一: 按照大的模块划分
|---src
|---pages
|---page-A
|---page-B
|---models
|---model-A.ts
|---model-B.ts
|---utils
|---util-A.ts
|---util-B.ts
模式二:内聚
|---src
|---pages
|---page-A
|---model-A.ts
|---util-A.ts
|---page-B
|---model-B.ts
|---util-B.ts
|---models
|---common-model.ts
|---utils
|---common-util.ts
大家更倾向哪一种目录架构呢?
谁能告诉我这两种目录架构有专业的名字吗?
1.7 ts keyof
解:
type IStatus = keyof typeof statusMap
1.8 不是本次需求的代码
这种大伙觉得需不需要本次改呢😌
2. 逻辑问题
2.1 后台管理项目也不能这么写呀
解:
tab_bar_style.style = {
...tab_bar_style.style,
color: 'blue',
background: 'red'
}
2.2 判断语句简写
?.
链式运算符就会去判断object 是否存在,不必要重复判断
2.3 return
2.4 循环
这里的问题在于没有保存之前的Key, 只需在第29行,将targetKey传递下去即可
2. 性能问题
2.1 没有分页的情况下,大数量渲染展示
(1) 原因一:vue无必要的数据劫持
在展示用户留言时,数据量达到10W级,此时vue会对数据做一个劫持,监听变化。 其实在我们这种纯渲染的场景是没有必要的。
所以可以直接冻结这个数据,防止vue做一个没必要的大数据量的劫持. 解:
const res = await fetch(`/api/user/list/message`)
return Object.freeze(res)
(2) 原因二:渲染用时长
数据量一大,需要生成的dom就会多,浏览器绘制时间就更长。
解:可以使用虚拟滚动来处理