日常开发为什么需要做Code Review

日常开发为什么需要做Code Review

一、背景

最近在开始一个新的项目，在查看项目中代码及具体细节时，发现这个项目真实一堆乱麻，没有规律可循，可总结下这个项目的缺陷

• 没有规律可循，没有结构性设计
• 不做公共封装，哪里方便哪里创建变量
• 一个个文件大于300行，都是超大文件，组件中方法超大逻辑
• class组件部分逻辑可继承封装，但是采取了复制粘贴相同代码的处理

看的我是一堆乱麻，不由得让我开始思考，程序员果真是懒，如果再勤快一些，可能会进行代码的封装和考虑这些公用方法及组件的抽取，同时也存在项目中Code Review 的缺失。

二、为什么需要Code Review 及优点

看这些奇奇怪怪的代码，不由得让我拍脑袋直言，当初多进行进行Code Review ，将这些问题屏蔽掉至少，那Code Review 有哪些优点呢

进行代码审查的原因可以归结为以下几点:

• 提高代码质量: 可以帮助开发者更好地编写代码，减少错误和缺陷，提高代码的质量和可维护性。通过Code Review ，开发者可以识别并修复潜在的问题，从而提高代码的可靠性和稳定性。

• 减少错误率：通过代码审查，开发人员可以发现之前可能没有注意到的错误或缺陷，从而减少错误的发生。

• 促进团队协作: 可以帮助团队成员之间更好地协作，不仅仅是老员工之间的协作，而且更加有效的帮助新员工融入到团队中，确保代码的一致性和可重用性。通过 Code Review ，开发者可以更好地理解其他开发者的代码，并从中学习，从而提高自己的编程水平。

• 提高开发效率: 可以帮助开发者更快地完成任务，减少代码编写的时间。通过 Code Review ，开发者可以更快地发现和解决问题，从而减少调试的时间和工作量。

• 确保代码符合规范: 可以帮助开发者遵循团队的代码规范，确保代码的可读性和可维护性。这对于大型项目的开发非常重要，因为遵循规范可以帮助开发者更好地协作和理解代码。

综上所述，Code Review 对于前端开发的质量和效率都非常重要。通过 Code Review ，开发者可以更好地编写代码，提高代码的质量和可维护性，促进团队协作，提高开发效率和确保代码符合规范。

三、Code Review 都有哪些方式

总体来说可分为常规代码Code Review 和 阻断式Code Review

• 常规代码Code Review
  提出修改建议与检查，不阻断代码合并或者影响开发者其他进度的检查
  通常这种检查会是团队成员聚集在一起，听其他成员讲述自己最近的开发代码。
• 阻断式Code Review
  需要合并进主分支等，需要必须要求复合规定时的检查，必须等待处理完所有建议或者不符合规定的点之后才能合并进主分支

四、Code Review 都会去注意哪些点

既然要做Code Review，那应该去检查哪些细节，我举个我开发中遇到的一些例子，给大家打个样，开发层更多地涉及到React Native中的一些知识点，可以选择性的阅读。

1. 代码是否按照lint规范提交，以及其他约定的一些规范提交

   至关重要的检查，lint可以省掉很多review内容，可能有时，一些比较着急的开发者会提交代码时使用-n命令忽略检查进行提交

2. useSelector使用规则，优化页面性能
   使用redux的同学可能会知道，useSelector会导致页面重新渲染，这边也就成为了日常review的一项

   const kfo = useSelector((state) => state.AtReducer.kfo || {}); 

   不规范原因： useSelector 底层是使用 === 比较 此处|| {}代码逻辑上容易触发render更新；
   建议改为以下写法

   const kfo = useSelector( (state) => state.AtReducer.kfo ) || {}
   

   使用useSelector时也需要关注state的粒度,导出自己需要的变量，不要一下子导出state.userInfo整个变量，同时使用shallowEqual进行比对处理

     const { username, age } = useSelector(state => { const { username, age } = state.userInfo;return { username, age }; }), shallowEqual);
   

3. 不要出现大文件，大文件增加分组进行拆分，按照业务模块分组或者extends继承机制处理

   超过1000行的文件出现bug，你会去看吗？你会很轻松的找到问题所在吗？答案肯定是不会，因为那么多行代码需要检查，而且也没有信息完全肯定能够修改正常，那肯定得避免出现这种文件，方便自己以及后面有其他同事接手这个项目时，能够不会有那么多的心理负担「🐶」

4. 双map或forEach遍历时，可能需要考虑优化

   出现这种情况的常见算法一般都是先找出数组中固定的某一个值，然后在用这个值去做业务逻辑，这样可能得和业务人员确定是否存在更加优化的处理方案，比如下面是用了es6中结构赋值的方案去减少了一次检索的过程，这种双循环的，一般都仔细检查下看看是否有优化方案

     // 使用解构赋值的模型，减少map循环的使用
   (lists || []).map(({ person: name }) => {// 使用name在进行处理其他业务逻辑// name
   })
   

5. 默认的空赋值[]或者{}

   正常来说，比如子组件的userList属性规定类型是数组，在父组件加工数据时提供数据默认值是非常好的习惯，于是我常常在组件内部或者mapStateToProps中看到类似的写法：

   const App = (props) => { // 当存在时赋予空数组，保证下层数组类型的正确性 const userList = props.userList || []; return (<Child userList={userList} />) 
   }; 
   

   当App多次渲染且props.userList为假值时，此时的userList也会被不断的赋予全新的空数组。还记得前文说的吗，当你结构没变化时，我们保证其引用不变不就好了，所以对于空数组都可以在全局赋予一个空数组，比如：

   const emptyArr = []; 
   const App = (props) => { // 当存在时赋予空数组，保证下层数组类型的正确性 const userList = props.userList || emptyArr;return (<Child userList={userList} />)
   }; 
   

   这样不管App如何渲染，当userList被赋予为空数组时也能让前后引用相同。

6. 使用 DeviceEventEmitter.addListener( ‘CHANGE’, function () {})注意依赖的项目可能不为最新值问题

   const ref = useRef();const listenerChange = () => {dispatch(FetchConfig({ jwtKey: userInfo?.jwtKey }, (dataList) => {setKols(dataList);}));}ref.current = listenerChange;useEffect((): function => {const listener = DeviceEventEmitter.addListener('CHANGE',function () {ref.current?.();})return () => {listener?.remove?.();};}, []);
   

   如果参数存在于延迟setTimeout,同时依赖于变量，也需要考虑通过以上方案处理,看看下面栗子🌰

   const [jwt] = useState('');setTimeout(() => {DeviceEventEmitter.emit('send',{ jwt: jwt })
   })
   

7. addListener事件命名建议采用【模块_特性_名称】，采用全局较为统一的规范
   如：LOGIN_STATUS_CHANGE,LOGOUT_STATUS_CHANGE

8. 使用useFocusEffect注意配合使用useCallback，否则会造成多次渲染

9. 默块引入使用路径别名，抽取行内样式

10. Platform.select 和 Platform.OS区分,针对平台比较复杂的逻辑，使用平台判断Platform.select的方案处理

11. react-native元素标签逻辑判断问题
    isShow逻辑判断使用!!isShow转义后进行使用，否则会提示文本必须在Text元素中使用的提示信息

    <View style={styles.fastContainer}>{!!isShow && <View></View>}
    </View>
    

12. 使用setTimeout、setInterval等伪代码时，需要在组件卸载时或关闭操作时将其清除掉
    ● setTimeout的销毁函数为clearTimeout
    ● setInterval的销毁函数为clearInterval
    对于多次打开的setTimeout，setInterval需要在再次开启时清空其值

    clearmyTime: () => void = () => {// eslint-disable-next-lineif (!!this.myTimeout) {clearTimeout(this.myTimeout);this.myTimeout = null;}
    };onSelected = () => {// 每次开启新的延迟计时器时，都进行清空操作this.clearmyTime();this.myTimeout = setTimeout(() => {// 操作}, 1000)
    }
    

13. 数组遍历需要考虑使用useMemo处理
    map，filter等方法，以及lodash中的遍历方法，都需要使用useMemo进行处理

14. 数值类比较操作 receive < 0 ? 0 : receive，可以考虑使用数学方法

    const value = receive < 0 ? 0 : receive;
    // 等同于
    const value = Math.max(receive, 0);
    

15. 保证key唯一，不要使用数组遍历产生的index，方便虚拟dom diff，提升性能

---