Skip to content

Conversation

asterism-q
Copy link

No description provided.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

在编辑器安装 prettiter 相关插件,并设置保存时自动格式化,避免这种问题

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码格式化一下

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没有用到的代码,可以删掉

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同理,注释的代码如无必要不要提交

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码格式化一下

@asterism-q
Copy link
Author

asterism-q commented Oct 15, 2021 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

一般来说,组件声明应规范化,比如加上强类型

const Component: React.FC<Props> = React.memo((props: Props) => {
  return xxx;
})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

注意代码健壮性,如

(overallData || []).map

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'YYYY-MM-DD' 等常量,可以统一写到一个文件中供复用

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果是空标签,这一行可以删掉

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

res?.data

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

企业实践中,console 调用一般禁止提交

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

建议组件内统一用箭头函数,语法更简单

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pageFlag * 10 及 pageFlag * 10 + maxOfpage - 1 等,建议在组件头部声明语义化的变量

slice 的结果也可以语义化声明出来,增加代码可读性

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

直接写label即可,不需要这样的封装,增加了缩进和代码复杂度

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果确实需要requie,应统一声明在文件头部

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里是否可以 import ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

对于业务逻辑,必要的地方加上注释增加代码可读性

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

数组判空,使用 lodash

_.isEmpty(renderList)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

数组迭代逻辑一般无需使用 for 循环。

推荐使用 ES6 的 Array.prototype.map 和 Array.prototype.reduce;其他API也可以了解下,如 filter some every forEach等 都比较常用

https://developer.mozilla.org/zh-CN/docs/Web/JavaScript/Reference/Global_Objects/Array/map

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if 语句,最好给每个条件后用花括号包裹,否则可能导致逻辑错误

Copy link

@Hungrated Hungrated Oct 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pop 方法会改变 selectedGoodsList 本身,不符合 React 数据流中的 immutable 原则;

这里可以直接根据需求在下一行 slice(0, _.size(selectedGoodsList)-1) 去掉最后一个,或 slice(1) 去掉第一个

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上,这种代码不要提交

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

数组长度用 _.size

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

取 scrollTop 可以抽象成一个通用方法

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const baseUrl = 'http://120.26.194.171:3000/v1';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants