在 Code.fun 做 Code Review(四)

时光如梭,一晃 2022 年已经过去 2/3,我们一起迎来 9 月。秋风送爽,丹桂漂亮,下面,我们一起回顾 8 月份我在 code.fun 完成的 Code Review 吧。

关于 Code Review 的介绍和经验,欢迎大家回顾前三篇,本文暂不重复:


0. 类和函数都要控制大小

这个问题也算常见。我们写代码的时候,有时候写着写着,某个类、函数越来越长。它们的内容可能很简单,比如 express.js 里处理请求的函数,就是各种验证,也不跟其它函数共享代码,但假如我们扎扎实实处理好每个传过来的参数,就会变得巨长。

通常来说,我的原则是:

  1. 出现一次,原样保留
  2. 出现两次,考虑提取成独立函数
  3. 出现 3+ 次,必须提取

于是面对这种巨型函数、巨型类,就会比较纠结。如果偏向我独立维护,那么我会维持原则,暂不重构;如果多人一起维护,那么我会建议进行提取和拆分。比如上面截图里的情况,由于流程较长,增加错误上报后,组件长度接近 1000 行,我就会建议负责的同事把新增代码放到独立的类里,然后通过继承的方式添加功能(我们用 vue-class-decorator 实现面向对象的代码风格)。

1. 确保主要功能可用

最近我们给产品中添加了 Sentry 来捕获错误,收获了不错的效果。以上代码出现在某位同事的 PR 中,它的目标是获取当前程序的执行环境,如果是开发环境,就不启动 Sentry,否则启动。

这段代码运行在 sketch 的 webview 里,我认为那里不算太可靠——即使是浏览器,我们也要小心 postMessage 之类的方法能否正常返回。所以,假如我们在返回后再初始化 Vue app,一旦中间出了问题,用户就会看到白屏。我认为这是不可接受的,相较于获取的价值,风险太大了。这段代码应该只负责 sentry 初始化就好。

1.1 副功能不应该影响主功能

这段代码跟上面的问题类似:错误上报不是产品的主要功能,用户并不会因为错误上报获得什么收益,反而如果错误上报失败,导致主要功能失效,用户会更反感。

这里还有个点:在异步函数里,如果我们不 await 异步函数调用,那么它的错误也不会影响当前异步函数的运行。所以,类似图中这种错误上报,就应该不 await,让它随便跑,大不了没数据,也好过影响用户。

2. 不应该直接操作父组件

这样的写法假定子组件充分知晓父组件的属性、方法,是很不好的习惯,会导致子组件和父组件深度耦合,影响复用和测试。这里其实换用事件 $emit 就可以了。

相对来说,父组件主动操作子组件比较容易接受,因为父组件通常知道它用到哪些子组件,这些子组件什么时候可用,等等。不过,我们仍然要尽量修改数据,而不是直接调用子组件的函数。这样才可以将组件解耦,方便维护和测试。

3. 少用 any

很多人开玩笑,说 TypeScript 写到后面就是 AnyScript,为了避免这种情况,我们应该尽可能少用 any。比如截图中的情况,我们明明知道后面要调用 .resetToDefault() 方法,也即知道这个组件的真实类型,那么这里就不应该用 any

4. 遵守“单一职责”原则

面向对象里有个“单一职责”原则,简单来说,一个类只应该负责一项主要工作;反过来,一个功能也应该集中在一处,不要东一块西一块,必须多个类凑在一起才能完成。

图上的代码里,本来 containerScaling 可以独立完成工作,但是这位同学把一个功能拆成了两个部分,一部分通过回调函数使用,就增加了错误发生的概率。实际上,这个 PR 合并后,又多打了好几次补丁,才算没有再发现错误。

5. computed 里不应该修改 data

Vue 里面,computed 是根据 data 数据的变化计算得来的。换言之,如果 comptued 里面又要计算 data,很容易导致死循环。我们要避免出现这个问题,就要避免在 computed 函数里给 data 属性赋值。

6. 杂项


7. 总结

如果诸位读者老爷对软件质量管理、软件开发效率、Code Review 有什么想问的、想说的,敬请在评论区与我互动。

我现在在 code.fun 工作,我们的产品会把设计稿自动转换成代码,期望大大提升前端的开发效率,如果你对这个产品感兴趣,欢迎联系我,或直接试用。我们公司提供远程工作岗位,有兴趣的同学可以联系我;朋友的公司 API7 也在招聘前端,有兴趣可以找我内推。

如果您觉得文章内容对您有用,不妨支持我创作更多有价值的分享:


已发布

分类

来自

评论

发表回复

您的电子邮箱地址不会被公开。 必填项已用 * 标注

此站点使用Akismet来减少垃圾评论。了解我们如何处理您的评论数据