Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: smaller package size #50

Merged
merged 6 commits into from
Oct 7, 2022
Merged

refactor: smaller package size #50

merged 6 commits into from
Oct 7, 2022

Conversation

qmhc
Copy link
Contributor

@qmhc qmhc commented Jul 5, 2022

在这个 PR 中我做了一些改进,以及一些自作主张的调整。

  • 改变了运算方法的创建方式,采用了一个构造方法来创建运算方法,并在这个过程中使用循环取代了递归
  • 将所有的方法的注释改为了英语,并补全了参数说明

首先关于第一点,我实施它是因为可以使得压缩 gzip 后包变得更小了:

之前:before

之后:after

同时在效率上能够有微弱的提升:

function test() {
  let times = 0
  for (let i = 0; i < 10; ++i) {
    let start = Date.now()
    NP.plus(...new Array(120000).fill(1))
    times += Date.now() - start
  }
  console.log(times / 10)
}

在 Chrome 下运行数次,递归方案平均时长约在 242ms,循环方案时长约在 235ms,不过这个性能差异在正常的使用中我觉得几乎没影响。

第二点是我自作主张的调整,我认为全面的参数说明和英语能够更广泛的适应更多用户,如果你不喜欢这样的调整,我可以撤回。

@codecov
Copy link

codecov bot commented Oct 7, 2022

Codecov Report

Base: 96.82% // Head: 94.87% // Decreases project coverage by -1.95% ⚠️

Coverage data is based on head (973458c) compared to base (e516fed).
Patch coverage: 94.44% of modified lines in pull request are covered.

❗ Current head 973458c differs from pull request most recent head a49a973. Consider uploading reports for the commit a49a973 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
- Coverage   96.82%   94.87%   -1.96%     
==========================================
  Files           1        1              
  Lines          63       78      +15     
  Branches       13       13              
==========================================
+ Hits           61       74      +13     
- Misses          2        4       +2     
Impacted Files Coverage Δ
src/index.ts 94.87% <94.44%> (-1.96%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

src/index.ts Outdated
others.forEach((num) => {
res = operation(res, num);
});
for (let i = 1, len = nums.length; i < len; ++i) {
Copy link
Member

@camsong camsong Oct 7, 2022

Choose a reason for hiding this comment

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

I like this idea very much, better to use array#reduce:

let [first, ...others] = numbers;
return others.reduce((prev, next) => operation(prev, next), first) as number;

@qmhc qmhc requested a review from camsong October 7, 2022 06:07
@camsong camsong merged commit 2783b57 into nefe:master Oct 7, 2022
@camsong camsong changed the title refacotr: smaller package size refactor: smaller package size Oct 7, 2022
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.

None yet

2 participants