前几天在技术群里聊起Code Review的事，大伙儿似乎都憋了一肚子气：

我觉得这份难言之隐应该要让更多人看到，就约了个稿：整理（脱敏）出了这篇小小的屎山合集，供大家品鉴。

1折磨人的 if else

可能存在下面一些问题

1. 过多的嵌套
2. 逻辑处理冗余
3. 没有做好防御编程（错误处理

直接来一个代码例子，这是一个获取背景颜色的方法，但是随着业务的不断变化，背景颜色的来源越来越多，在一些业务人员的处理下可能是这样的：

相信你在读上面的代码的时候是极为痛苦的，想要一目了然的知道最终会进入哪个分支，基本不可能。于是基于下面两个原则

• 合理的抽取成函数
• 错误优先返回

有了一个基础版本的重构：

2. useEffect不指定依赖

依赖参数缺失。

javascript复制代码useEffect(() => {
    console.log('no deps=====')
    // code...
});

这样的话，每次页面有重渲染，该useEffect都会执行，带来严重的性能问题。例如我们项目中，这个useEffect内部执行的是第一点中的内容，即每次都会绑定一个scroll事件的回调，而且页面中有实时轮询接口每隔5s刷新一次列表，用户在该页面稍加停留，就会有卡顿问题出现。解决方案很简单，根据useEffect的回调函数内容可知，如果需要在第一次渲染之后挂载一个scroll的回调函数，那么就给useEffect第二个参数传入空数组即可，参考官方文档(react.dev/reference/r…

scss复制代码useEffect(() => {
    // code...
}, []);

3. 硬编码

硬编码，即一些数据信息或配置信息直接写死在逻辑代码中，例如

这两行代码本意是从url上拿到指定的参数的值，如果没有，会用一个固定的配置做兜底。

乍一看代码逻辑很清晰，但再想深一层，兜底值具体的含义是什么？为什么要用这两个值来兜底？写这行代码的同学可能很快可以解答，但是一段时间之后，写代码的人和提需求的人都找不到了呢？

这个示例代码还比较简单，拿对应的值去后台可以找到对应的含义，如果是写死的是枚举值，而且还没有类型定义，那代码就很难维护了。

解决此类问题，要么将这些内容配置化，即写到一个config文件中，使用清晰的语义化命名变量；要么，至少在硬编码的地方写上注释，交代清楚这里需要硬编码的前因后果。

沐洒：

关于硬编码问题，我在之前的一篇关于“配置管理”的文章里有详细阐述和应对方案，感兴趣的朋友可以看看《小白也能做出满分前端工程：01 配置管理》

4. 放任文件长度，只着眼于当下的需求

很多同学做需求、写代码都比较少从全局考虑，只关注到当前需求如何完成。从“战术”上来说没有问题，快速完成产品的需求、快速迭代产品也是大家希望看到的。

可一旦只关注“战术实现”而忽略“战略设计”，除非做的产品是月抛型的，否则一定会遇到旧逻辑难以修改的情况。

如果再加上一个文件被多达10余人修改过的情况，那么每改一行代码都会是一场灾难，例如最近接手的一个页面：

单文件高达1600多行！哪怕去除300多行的注释，和300多行的模板，剩下的逻辑代码也有1000行左右，这种代码可读性就极其糟糕，必须进行拆分。

而很常见的是，由于每一任经手人都疏于考虑全局，导致大量代码毫无模块化可言，甚至出现多个useEffect的依赖是完全相同的：

这里明显还有另一个问题：滥用hooks。

从行号可以看出来确实是相同的依赖写了多个useEffect，很明显是多个同学各写各的的需求引入的这些hooks。
这代码跑肯定是能跑的，但是很可能会出现多个hooks中修改同一个变量，导致其他地方在使用的时候需要搞一些很tricky的操作来修Bug。

5.变量无初始值

在typescript的加持下，对变量的类型定义可以说是日益严格了。可是在一些变量的类型定义比较复杂的情况下，可能一个变量的字段很多、层级很复杂，此时有些同学就可能想偷个懒了，例如：

scss复制代码const [variable, setVariable] = useState<ComplicatedType>();

// some code...
const queryData = function() {
    // some logic
    setVariable({ show: true });
};

useEffect(() => {
    queryData();
}, []);

return variable.show ? <Component /> : null;

这里的问题很明显，如果queryData耗时比较长，在第一次渲染的时候，最后一行的variable.show就会报错了，因为variable的初始值是undefined。所以声明变量时，一定要根据变量的类型设置好有效默认值。

6. 三元选择符嵌套使用

网上很多人会推荐说用三元选择符代替简单的if-else，但几乎没有见过有人提到嵌套使用三元选择符的事情，如果看到如下代码，不知道各位读者会作何感想？

json复制代码{condition1 === 1
    ? "数据加载中"
    : condition2
    ? "没有更多了"
    : condition3
    ? "当前没有可用房间"
    : "数据加载中"}

真的很难理解，明明只是一个简单的提示语句的判断，却需要拿出分析性能的精力去理解，多少有点得不偿失了。

这还只是一种比较简单的三元选择符的嵌套，因为当各个条件分支都为true时，就直接返回了，没有做更多的判断，如果再多做一层，都会直接把人的cpu的干爆炸了。

替代方案：

1. 直接用if-else，可读性更高，以后如果要加逻辑也很方便。
2. Early Return，也叫卫语句，这种写法能有效简化逻辑，增加可读性。

kotlin复制代码if (condition1 === 1) return "数据加载中";
if (condition2) return "没有更多了";
if (condition3) return "当前没有可用房间";
return "数据加载中";

虽然不嵌套的三元选择符很简单，但是在例如jsx的模版中，仍然不建议大量使用三元选择符，因为可能会出现如下代码：

css复制代码return (
    condition1 ? (
        <div className={condition2 ? cls1 : cls2}>
            {condition3 ? "111" : "222"}
            {condition4 ? (
                <Component prop1={condition5 ? a : b} />
            ) : null
        </div>
    ) : (
        <Component2>
            {condition6 ? children1 : children2}
        </Component2>
    )
)

类似的代码在我们的项目中频繁出现，模版中大量的三元选择符导致文件内容拉得很长，很容易看着看着就不记得自己在哪个逻辑分支上了。

像这种简单的三元选择符，做成一个简单的memo变量，哪怕是在组件内直接写变量定义（例如：const clsName = condition2 ? cls1 : cls2），最终到模板的可读性也会比上述代码高。

7. 逻辑不拆分

React hooks可以很方便地帮助开发者聚合逻辑抽离成自定义hooks，千万不要把一个页面所有的useState、useEffect等全都放在一个文件中：

其实从功能上可以对页面进行拆分，拆分之后这些变量的定义也就可以拆出去了。其中有一个很简单的原则就是，如果一个逻辑同时涉及到了useState和useEffect，那么就可以一并抽离出去成为一个自定义hooks。例如接口请求大家一般都是直接在业务逻辑中做：

scss复制代码const Comp = () => {
    const [data, setData] = useState({});
    const [loading, setLoading] = useState(false);
    
    useEffect(() => {
        setLoading(true);
        queryData()
            .then((response) => {
                setData(response);
            })
            .catch((error) => {
                console.error(error);
            })
            .finally(() => {
                setLoading(false);
            });
    });
    
    if (loading) return "loading...";
    
    return <div>{data.text}</div>;
}

根据上面的原则，和数据拉取相关的内容涉及到了useState和useEffect，这整块逻辑就可以拆出去，那么最终就只剩下：

kotlin复制代码const Comp = () => {
    const { data, loading } = useQueryData();
    
    if (loading) return "loading...";
    
    return <div>{data.text}</div>;
};

这样下来，Comp组件就变得身份清爽了。大家可以参考阿里的ahooks库，里面收集了很多前端常用的hooks，可以极大提升开发效率和减少重复代码。

8. 随意读取window对象的值

作为大型项目，很容易需要依赖别的模板挂载到window对象的内容，读取的时候需要考虑到是否有可能拿不到window对象上的内容，从而导致js报错？例如：

go复制代码window.tmeXXX.a.func();

如果这个tmeXXX所在的js加载失败了，或者是某个版本中没有a这个属性或者func这个函数，那么页面就会白屏。

好啦，最近CR常出现的8种屎山代码都讲完了，你写过哪几种？你们团队的代码中又有哪些让你一口老血喷出来的不良代码呢？欢迎评论区告诉我。

写出高质量代码的Tips

打好基础

写出高质量代码，并不是搭建空中楼阁，需要有一定的基础，这里我重点强调与代码质量密切相关的几点：

• 掌握好开发语言，比如做Android就必须对Java足够熟悉，《Effective Java》一书就是教授大家如何更好得掌握Java, 写出高质量Java代码。
• 熟悉开发平台, 不同的开发平台，有不同的API, 有不同的工作原理，同样是Java代码，在PC上写与Android上写很多地方不一样，要去熟悉Android编程的一些特性，iOS编程的一些特性，了解清楚这些，才能写出更加地道的代码，充分发挥各自平台的优势。
• 基础的数据结构与算法，掌握好这些在解决一些特定问题时，可以以更加优雅有效的方式处理。
• 基础的设计原则，无需完全掌握23种经典设计模式，只需要了解一些常用的设计原则即可，甚至你也可以只了解什么是低耦合，并在你的代码中坚持实践，也能写出很不错的代码。

2. 代码标准

代码标准在团队合作中尤为重要，谁也不希望一个项目中代码风格各异，看得让人糟心，即便是个人开发者，现在也需要跟各种开源项目打交道。标准怎么定是一个老生常谈的话题，我个人职业生涯中经历过很多次的代码标准讨论会议，C++, C#, Java等等，大家有时会坚持自己的习惯不肯退让。可现如今时代不一样了，Google等大厂已经为我们制定好了各种标准，不用争了，就用这些业界标准吧。

3. 想好再写

除非你很清楚你要怎么做，否则我不介议边做边想。
你真的搞清楚你要解决的问题是什么了吗？你的方案是否能有效？有没有更优雅简单的方案？准备怎么设计它，必要的情况下，需要有设计文档，复杂一些的设计需要有同行评审，写代码其实是很简单的事情，前提是你得先想清楚。

4. 代码重构

重构对于代码质量的重要性不言而喻，反正我是很难一次把代码写得让自己满意、无可挑剔，《重构》这本书作为业内经典也理应人人必读，也有其他类似的教授重构技巧的书，有些也非常不错，遗憾的是我发现很多工作多年的同学甚至都没有了解过重构的概念。

5. 技术债务

知乎上最近有个热门问题《为什么有些大公司技术弱爆了?》，其实里面提到的很多归根结底都是技术债务问题，这在一些大公司尤为常见。技术债务话题太大，但就代码质量而言，我只想提一下不要因为这些债是前人留下的你就不去管，现实是没有多少机会让你从一个清爽清新的项目开始做起，你不得不去面对这些，你也没法完全不跟这些所谓的烂代码打交道。

因此我建议各位：当你负责一个小模块时，除了把它做好之外，也要顺便将与之纠缠在一起的技术债务还掉，因为这些债务最终将是整个团队来共同承担，任何一个人都别想独善其身，如果你还对高质量代码有追求的话。

作为团队的技术负责人，也要顶住压力，鼓励大家勇于做出尝试，引导大家不断改进代码质量，不要总是畏手畏脚，停滞不前，真要背锅也得上，要有担当。

6. 代码审查

我曾经听过一些较高级别的技术分享，竟然还不时听到一些呼吁大家要做代码审查的主题，我以为在这个级别的技术会议上，不应再讨论代码审查有什么好，为什么要做代码审查之类的问题。同时我接触过相当多所谓国内一线互联网公司，竟有许多是不做代码审查的，这一度让我颇为意外。

这里也不想多谈如何做好代码审查，只是就代码质量这点，不客气地说：没有过代码审查经历的同学，往往很难写出高质量的代码，尤其是在各种追求速度的糙快猛创业公司。

7. 静态检查

很多代码上的问题，都可以通过一些工具来找到，某些场景下，它比人要靠谱得多，至少不会出现某些细节上的遗漏，同时也能有效帮助大家减少代码审查的工作量。

Android开发中有Lint, Find bugs, PMD等优秀静态检查工具可用，通过改进这些工具找出的问题，就能对语法的细节，规范，编程的技巧有更多直观了解。

建议最好与持续集成(CI)，代码审查环境配套使用， 每次提交的代码都能自动验证是否通过了工具的代码检查，通过才允许提交。

8. 单元测试

Android单元测试，一直备受争议，主要还是原生的测试框架不够方便，每跑一次用例需要在模拟器或者真机上运行，效率太低，也不方便在CI环境下自动构建单元测试，好在有Robolectric，能帮我们解决部分问题。

单元测试的一个非常显著的优点是，当你需要修改大量代码时，尽管放心修改，只需要保证单元测试用例通过即可，无需瞻前顾后。

9. 充分自测

有一种说法：程序员最害怕的是他自己写的代码，尤其是准备在众人面前show自己的工作成果时，因此在写完代码后，需要至少跑一遍基本的场景，一些简单的异常流。在把你的工作成果提交给测试或用户前，充分自测是基本的职业素养，不要总想着让测试帮你找问题，随便用几下就Crash的东西，你好意思拿给别人吗？

10. 善用开源

并非开源的东西，质量就高，但至少关注度较高，使用人数较多，口碑较好的开源项目，质量是有一定保证的，这其中的道理很简单。即便存在一些问题，也可以通过提交反馈，不断改进。最重要的是，你自己花时间造的轮子，需要很多精力维护，而充分利用开源项目，能帮助你节省很多时间，把精力专注在最需要你关心的问题上。