业务代码codereview规范

忘是亡心i 2022-11-07 00:59 174阅读 0赞

### 为什要要codereview ###

新功能开发好上线前，一般都要进行codereview，在codereview过程中，可以发现一些问题，提早规避线上问题或故障；也可以规范代码，大家互相学习和进步。

### codereview原则 ###

1.  先看代码可读性和规范性，其次再考虑抽象和复用，最后考虑性能
2.  comments不能省略，留下记录很重要，最好是面对面codereview
3.  每次review的规模不应该太大，200行左右，不要超过800行
4.  虚心接收别人的comments

### 常规问题检查 ###

*  代码是否能实现预期功能
 *  代码格式是否规范
 *  变量名是否易懂
 *  不要无用代码
 *  对于边界值的处理，例如 null, undefined等的兜底逻辑
 *  代码是否有可读性，是否可以优化
 *  代码结构分层是否合理

--------------------

以下记录平时codereview时看到的代码，以做学习借鉴

bad code

#### 1. 代码无法达到预期 ####

**存在问题**：当`format`方法返回无效值（`NaN, undefined, null`等）时，text结果只是`¥`，但实际上不会  
![在这里插入图片描述][watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70]  
**原因**：运算符`+`的优先级高于`||`，如下：  
![在这里插入图片描述][watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70 1]  
**修改方法**：

1.  使用`()`，让`||`运算先执行

1.  在`format`方法中做好兜底处理

#### 2. 未清除定时器 ####

查询方法需要延时1秒执行，用`setTimeout`实现，却没有清除定时器，如下:  
![在这里插入图片描述][watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center]  
**修改方案**：在func方法里先检查并清除定时器，在回调方法里也及时清除一下  
![在这里插入图片描述][watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70 2]

#### 3. 不必要的if判断 ####

以下代码，在riskStatus不同值的时候，需要给request对象添加不同的属性，写了4个if来判断  
![在这里插入图片描述][watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 1]  
**存在问题**：

1.  无论riskStatus取值多少，4个if判断都会执行一遍，效果跟switch…case没加break一样；虽然不会发生错误，但是会做不必要的if判断。
2.  if判断语句里用到的1，2，3，4可以用枚举，易读性更强且好维护

**修改方案**：

1.  用eles if改写if  
    ![在这里插入图片描述][watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 2]
2.  采用状态模式  
    ![在这里插入图片描述][watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 3]

#### 4. 关于类型转换 ####

存在问题：额外转换一次布尔类型, `riskStatus === 1 || riskStatus === 4`的返回值就是布尔类型  
![在这里插入图片描述][watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 4]

#### 5. 使用es新语法，简化代码 ####

**存在问题**： 补全时间时，判断是否小于10，手动在头部补全0  
![在这里插入图片描述][watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 5]  
修改方案：  
ES2017新语法 `string.padStart(str, len)`，可以自动在头部补全0，省去`if`判断

good code

#### 1. 功能相似的按钮通过数组实现 ####

表格最后一列是操作列，展示操作按钮【编辑】、【查看】，功能类似，通过枚举和数组渲染按钮，减少代码量，可维护性高。  
![在这里插入图片描述][watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 6]

[watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70]: /images/20221023/94b75766ed0a41a694d7567b8755f1f1.png
[watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70 1]: /images/20221023/c71f7701bb1141bcb758a8ba55fbcb82.png
[watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center]: /images/20221023/6400996f94684cfba4dcaed9b3dbb036.png
[watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70 2]: /images/20221023/7597a90c2f0a4ba1b3ff9495ee17ce87.png
[watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 1]: /images/20221023/e06acf75d0f74be081fc17acb4988525.png
[watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 2]: /images/20221023/921bed991af04526913a623476b495e4.png
[watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 3]: /images/20221023/3bf972b903714bcda07b4d87d2f5c9fd.png
[watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 4]: /images/20221023/eaa8c4136b1448628b6c32f35b1cc0dd.png
[watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 5]: /images/20221023/935535c507344545a5ebe7d399aa96fd.png
[watermark_type_ZmFuZ3poZW5naGVpdGk_shadow_10_text_aHR0cHM6Ly9ibG9nLmNzZG4ubmV0L2hhb3lhbnl1Xw_size_16_color_FFFFFF_t_70_pic_center 6]: /images/20221023/f2d0e9baf37042e3a2f583215f2a589a.png