Code Review 总结前言编程是一种工艺，刚开始编程时，最好的学习方式是能够作为学徒，参与由软件熟练工指导的正式

1. 前言

编程是一种工艺，刚开始编程时，最好的学习方式是能够作为学徒，参与由软件熟练工指导的正式学徒训练。

现实中，多数人都只能在不太理想的环境中训练自己，一直埋头苦干，编写代码时缺乏思考。

如何从编写可用的代码到正确的代码、甚至是优秀的代码呢？

很多开发习惯、代码细节也不知道写得好还是不好，不知道怎么提升自己的编码能力？

代码检视就是一个很好的途径，加入一个正规的团队，有 code Reviewer 和 refactor。

本文是我近期代码检视的总结。

文章目录

源代码的版面和样式
有意义的事物起有意义的名称
样式布局
性能优化

2. 源代码的版面和样式

2.1 代码组织

改前

问题

在页面层级的目录里写一个组件，到底是想干嘛

为什么之前会这样做

没有考虑那么多，顺手就建立了。

建议

非页面组件不应该放到页面层级目录下，这样后续维护起来很混乱。要时刻思考，这是一个多人协作的项目。

2.2 引入文件规范

改前

以下代码摘自所做的 taro 小程序项目中的一小段：

import { Modal } from '../../../../components';
import { useClassnames, useStateHook, COLOR } from '../../../../utils';
import { RequiredTag } from "../../../../components/required-tag"
import edit from '../../../../images/order/edit@3x.png';
import Arrow_Icon from '../../../../images/order/arrow@2x.png';
import FRAGILE_Icon from '../../../../images/order/fragile@2x.png';

import { InsuranceEntryProps, InsuranceLineState, InsurancePackageProps, InsurancePackageState, ShowText, IInsurancePackage } from "./index.d"
import './index.scss';

export type InsuranceEntryMode = 'single' | 'multi' | ''; // 单个入口 | 保价套餐

问题

源文件中引入的依赖顺序不规范
类型文件有本文件中声明的「InsuranceEntryMode」，也有从「index.d.ts 」中引入。

建议

遵循一致性原则，同类型的文件放在一起，并按照以下的引入顺序：

外部 npm 包（Taro、React...）
通用组件
业务组件
Services 服务
utils 方法，归类分组
样式文件
静态文件，比如 icon 图片

改后

把「InsuranceEntryMode」加入到 index.d.ts 中

// index.d.ts
//...
export enum InsuranceEntryMode {
	single = 'single', // 单个入口
	multi = 'multi', // 保价套餐
}

组件 components 中提供 index 统一的出口，utils 归类分组。


// index.tsx
import { Modal, RequiredTag } from '../../../../components';
import { useClassnames, useStateHook } from '../../../../utils/userHooks';
import { COLOR } from '../../../../utils/constants';

import { 
  InsuranceEntryProps, 
  InsuranceLineState, 
  InsurancePackageProps, 
  InsurancePackageState, 
  ShowText, 
  InsurancePackage, 
  InsuranceEntryMode 
} from "./index.d"
import './index.scss';

import ARROW_ICON from '../../../../images/order/arrow@2x.png';
import FRAGILE_ICON from '../../../../images/order/fragile@2x.png';
import EDIT_ICON from '../../../../images/order/edit@3x.png';
```DIT_ICON from '../../../../images/order/edit@3x.png';

3. 有意义的事物起有意义的名称

3.1 变量命名

改前

const deepEqual = (object1: Object, object2: Object):boolean => {
    `const key1s = getObjectKeys(object1);` 
    `const key2s = getObjectKeys(object2);`

    if (key1s.length !== key2s.length) {
        return false;
    }

    for (const key of key1s) {
        const val1 = object1[key];
        const val2 = object2[key];
        const areObjects = isObject(val1) && isObject(val2);
        if (areObjects && !deepEqual(val1, val2) || 
            (!areObjects && val1 !== val2) 
        ) {
            return false;
        }
    }
    return true;
}

问题

这里的 key1s 和 key2s 给人的感觉就是，key1 的数组，key2 的数组。

之前为什么这样做

根本原因是没有认真去想好。

建议

key1s => keysOfObject1

key2s =>keysOfObject2

多想想，高工都是怎么命名的，他是怎样思考的。

const deepEqual = (object1: Object, object2: Object):boolean => {
    const keysOfObject1 = getObjectKeys(object1);
    const keysOfObject2 = getObjectKeys(object2);

    if (keysOfObject1.length !== keysOfObject2.length) {
        return false;
    }

    for (const key of keysOfObject1) {
        const val1 = object1[key];
        const val2 = object2[key];
        const areObjects = isObject(val1) && isObject(val2);
        if (
          	areObjects && !deepEqual(val1, val2) || 
            (!areObjects && val1 !== val2) 
        ) {
            return false;
        }
    }
    return true;
}

3.2 函数命名

改前

const renderInsurance = useMemo(() => {
    return (
        <View>
        // ....
    	</View>
    );
}, []);

问题

render 应该是一个动词，但这里返回的是一个静态的 jsx。

建议

最起码用 useCallback 表示是一个方法一个动作。

改后

const renderInsurance = useCallback(() => {
    return (
        <View>
            // ....
    	</View>
    );
}, []);

3.3 任何时候都不要使用拼音命名

改前

问题

拼音是极不推荐的名字方式，中文多音字很多，用拼音命名会造成歧义；

也会跟其他英文命名的代码有严重的不一致性。

之前为什么这样做

这些都是实际的产品分类不知怎么命名好，觉得英文会有不够准确对应的值。

建议

采用英文并不会产生歧义不够准确，可以添加上对应的注释，其他程序员如果不清楚可以直接使用翻译软件。

把所有的产品类型都放到一个 productCode 枚举类型下进行引用，如果要改，就把整个项目涉及到文件都全部改好。

改后

export interface PRODUCT_TYPE {
    STANDARD = '101', // xxx
    HEAVY = '102', // xxx
    STANDARD_LAND = '103' // xxx
}

4. 样式布局

4.1 不要以小包大

改前

问题

这里实现的效果时，点击一个小的 tag 标签，唤起一个弹框，这个弹框是铺满整个页面的。

整体的页面布局上不能以小包大。除非唤起的面板只是类似下拉框的一小部分。

之前为什么这样做

没有遵循具体的章法，只是借鉴下拉框的实现思路。

建议

把 Modal 移出去，点击 tag 时通过 showModal 这样的形式来显示 Modal。

改后

const $FragileRule = useMemo(
        () => (
            <Fragment>
                <View
                    className={classnames('package-fragile-tag-modal-header')}
                >
                    <Text
                        className={classnames(
                            'package-fragile-tag-modal-header-title'
                        )}
                    >
                        易碎保收费说明
                    </Text>
                </View>
                <View
                    className={classnames('package-fragile-tag-modal-content')}
                >
                    <View
                        className={classnames(
                            'package-fragile-tag-modal-content--1'
                        )}
                    >
                        您邮寄的物品属于易碎类！
                    </View>
                    <View
                        className={classnames(
                            'package-fragile-tag-modal-content--2'
                        )}
                    >
                        保价费用 = 物品价格 * 8%
                    </View>
                    <View
                        className={classnames(
                            'package-fragile-tag-modal-content--3'
                        )}
                    >
                        其中时效及特惠产品最低收费2元/票、快运标准达最低收费3元/票起
                    </View>
                </View>
            </Fragment>
        ),
        []
    );
    // 易碎保标签
    const $FragileTag = useMemo(() => (
        <View className={classnames('package-fragile-tag')}>
            <View
                onClick={() => {
                    showModal({
                        jsx: $FragileRule
                    })
                }}
                className={classnames('package-fragile-tag-toogle')}
            >
                <Image
                    className={classnames('package-fragile-tag-icon')}
                    src={FRAGILE_ICON}
                />
                <Text className={classnames('package-fragile-tag-text')}>
                    易碎保
                </Text>
                <Image
                    src={ARROW_ICON}
                    className={classnames('package-fragile-tag-arrow')}
                />
            </View>
        </View>
    ), []);

5. 性能优化

5.1 避免 jsx 绑定箭头函数

改前

问题

使用箭头函数会导致每次渲染都创建新的函数，就会对前一个函数进行垃圾回收，这样会对垃圾回收器造成负担。

箭头函数也会影响子组件的渲染过程，因为传入的函数 prop 每次都改变，

这样对于使用 React.memo 包裹的子组件没有起作用。

除非是遇到动态传入参数的情况，比如下面这种：

<View 
    className={classnames({
        'tab-nav-item': true,
        'tab-nav-item--active':
            activeTabKey === TAB_BAR.AVAILABLE,
    })}
    onClick={() => handleClickTabBar(TAB_BAR.AVAILABLE)}
>
    可使用（{availbaleCount}）
</View>

之前为什么这样做

没有考虑，觉得方便就写了。

建议

const handleClickFragileTag = useCallback(() => {
    showModal({ jsx: $FragileRule }));
}, []);

改后

const $FragileTag = useMemo(() => (
    <View 
        onClick={handleClickFragileTag} 
        className={classnames('package-fragile-tag')}
    >
        <View >
            //...
        </View>
    </View>
), []);

5.2 请求数据缓存

改前

// 全局数据
const globalData = {
    systemInfo: {},
    // 定制头部的一个信息
    customHeader: {
        contentHeight: 54,
        statusBarHeight: 20,
        height: 64
    },
    host,
    apiGateWay,
    // ..
    user: {
        authStatus: 'unAuth' // 授权状态 ---- 默认未授权
    },
    version,
    ...authConfig[process.env.NODE_ENV],
    orderCoupon: { // 下单优惠券列表
        // availbaleCount: 0,
        // availableCouponList: [],
        // unAvailableCouponList: [],
        // unAvailbaleCount: 0,
    }
}

问题

下单页面会请求优惠券接口数据，优惠券页面也会请求同一接口数据，

直接把数据缓存到 globalData 里，这是不合理的。因为优惠券的数据不属于全局数据。

这样还污染到了，需要在调用优惠券的接口写入缓存，在二次使用的页面比如优惠券页面获取缓存，都需要加入对应的判断代码。

另外这里也不像 redux 使用统一的接口，统一更新数据源。

还有就是请求的优惠券输入参数不同，也使用到了相同的缓存。

// 下单页面写入
const [availableCouponRes, unAvailableCouponRes] = 
    await Promise.all([
        fetchAvailableCoupon(param), 
        fetchUnAvailableCoupon(param)
    ]);

globalData.orderCoupon = {
    availbaleCount,
    availableCouponList: availableCouponList || [],
    unAvailableCouponList: unAvailableCouponList || [],
    unAvailbaleCount,
}

// 优惠券页面读取
const { orderCoupon } = globalData;
if (Object.keys(orderCoupon).length > 0) {
    initCouponList(orderCoupon);
} else {
  // ...
}

建议

直接在 api 请求层面来做缓存，需要为下单可用券和下单不可用券两个接口都做缓存处理。

对输入的参数进行深比较，如果一致就进行缓存。

考虑同时两个以上的调用此 api，可用 promise 来处理，避免并发时实际上还是请求了两次数据。

之前为什么这样做

由于项目中没有使用 redux 类似的方案全局缓存数据。

疑问

需要考虑过期时间吗？优惠券的数据什么时候过期？

假如入参数据一致，但是优惠券的数据在后台已经设置，该如何处理。

就是说用户在使用小程序期间，后台数据已经更新。

回答

接口缓存的前提是，相同的输入，相同的输出。

至于缓存中遇到数据的更新机率非常小，很少有人一直玩小程序获取优惠券。
小程序退出时，直接清除内存就好。这种情况不需要考虑过期的问题。

改后

工具函数：

// utils.ts
interface PromiseApi {
    (...args: any): Promise<any>;
};
const promiseCacheMap = new Map();
export const withPromiseCache = async (
    promiseApi: PromiseApi,
    options: { args: any[], ignoreCompare?: boolean; }
) => {
    const {
        args = [],
        ignoreCompare = false // 是否忽略比较参数
    } = options;
    let promise: Promise<any>
    const cache = promiseCacheMap.get(promiseApi);
    if (cache) { // 有缓存
        const { args: cacheArgs, promise: cachePromise } = cache;
        if (
            ignoreCompare ||  // 忽略 args 参数依赖
            compareArray(args, cacheArgs) // args 依赖相同
        ) {
            return cachePromise;
        }
    }
    // 没有缓存记录或是没命中缓存
    promise = promiseApi(...args);
    // 记录缓存
    promiseCacheMap.set(promiseApi, { args, promise });
    return promise;
};

export const withCache = (...options) => {
    const [promiseApi, ...args] = options;
    return withPromiseCache(promiseApi, { args });
};

请求接口：

// services.ts
export const fetchAvailableCoupon = 
    async (options: IOrderCouponListOption) => 
        fetchApiJson({
            data: options,
            url: COUPON_AVAILBALE,
            errorMessage: '请求优惠券列表异常',
            errorCode: 'FETCH_COUPON_AVAILBALE_FAIL',
        });
fetchAvailableCoupon.withCache = 
    async (options: IOrderCouponListOption) => 
        withCache(fetchAvailableCoupon, options);

组件使用：

const [availableCouponRes, unAvailableCouponRes] =
    await Promise.all([
        fetchAvailableCoupon.withCache(option),
        fetchUnAvailableCoupon.withCache(option),
    ]);

上面的代码没有考虑到在后端出错的情况下对 promise 进行了删除，会出现第一个请求返回错误后（404）。

然后网络正常时，由于请求相同的参数会命中缓存，而此时的 Promise 是错误的。

那么如果只缓存成功的 Promise，失败直接重新请求，是否可行呢？

回答是不行的，因为如果第一个请求还在 pending中。这时候还没有缓存，后续的请求不会命中缓存，一样会发起多个请求的。

只能保证第一个请求发出缓存后，后续的请求再进行请求。

更好的方式是使用一个队列维护所有请求的 Promise executor，具体是：

每个请求都返回一个新的Promise, Promise的exector的执行时机，通过一个队列保存。
当队列长度为1的时候，执行一次请求，如果请求成功，那么遍历队列中的exector，拿到请求的结果然后resolve。
如果请求失败了，那么就把这个Promise reject掉，同时出栈。然后递归调用next
直到exector队列清空为止

const cacheAsync = function(promiseGenerator, cacheKey)  {
    const cache = new Map();
    const placeholder = Symbol();
    return async (params) => {
        return new Promise((resolve, reject) => {
            cacheKey = cacheKey || params;
            let cacheConfig = cache.get(cacheKey);
            if (!cacheConfig) {
                // 没有命中
                cacheConfig = {
                    hit: placeholder,
                    exector: [{ resolve, reject }]
                };
                cache.set(cacheKey, cacheConfig);
            } else {
                // 命中缓存
                // 如果已经有命中的值，直接返回
                if (cacheConfig.hit !== placeholder) {  
                    return resolve(cacheConfig.hit); 
                }
                // 把当前的 Promise resolve、reject 存入队列
                cacheConfig.exector.push({ resolve, reject }); 
            }
            
            const { exector } = cacheConfig; 

            console.log('exector', exector.length);
            // 处理并发，在请求还处于 pending 过程中就发起了相同的请求
            // 只拿第一个 Promise 作为请求入口
            if (exector.length === 1) {
                const next = async () => {
                    try {
                        if (!exector.length) return;
                        const response = await promiseGenerator(params);
                        // 如果成功了，那么直接 resolve 掉剩余同样的请求
                        while(exector.length) { // 清空
                            exector.shift().resolve(response);
                        }
                        // 缓存结果
                        cacheConfig.hit = response;
                    } catch(error) {
                        // 如果失败了，那么这个 promise 的则为 reject
                        const { reject } = exector.shift();
                        reject(error);
                        next(); // 失败重试，降级为串行
                    }
                };
                next();
            }
        })
    }
}; 

async function fetchData() {
    const res = await fetch("http://localhost:3000/test");
    const data = await res.json();
    return data;
}

const fetchData = cacheAsync2(fetchData);

测试并发 Demo：github.com/naluduo233/…

6. 小结

养成良好的开发习惯，多思考，每一次的 Code Review 都是难得的学习机会。

每一次提交 Commit 之前，自己都应该先检视一遍，多注意一些细节上的问题。

Code Review 总结

1. 前言

2. 源代码的版面和样式

2.1 代码组织

2.2 引入文件规范

3. 有意义的事物起有意义的名称

3.1 变量命名

3.2 函数命名

3.3 任何时候都不要使用拼音命名

4. 样式布局

4.1 不要以小包大

5. 性能优化

5.1 避免 jsx 绑定箭头函数

5.2 请求数据缓存

6. 小结

参考资料