discord.js icon indicating copy to clipboard operation
discord.js copied to clipboard

feat: add `Collection#subtract()`

Open Syjalo opened this issue 2 years ago • 7 comments

Please describe the changes this PR makes and why it should be merged:

This PR adds a new method. The subtract method returns a new structure containing items where the keys and values of original structure are not present in the other.

Visualization

image

Status and versioning classification:

  • I know how to update typings and have done so, or typings don't need updating
  • This PR changes the library's interface (methods or parameters added)

Syjalo avatar Jul 29 '22 16:07 Syjalo

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Updated
discord-js ⬜️ Ignored (Inspect) Nov 19, 2022 at 9:27PM (UTC)
discord-js-guide ⬜️ Ignored (Inspect) Nov 19, 2022 at 9:27PM (UTC)

vercel[bot] avatar Jul 29 '22 16:07 vercel[bot]

This should be called difference instead of missing since it aligns with the mathematical terminology.

suneettipirneni avatar Jul 29 '22 16:07 suneettipirneni

This should be called difference instead of missing since it aligns with the mathematical terminology.

Actually, that method seems to already exist.

Jiralite avatar Jul 29 '22 17:07 Jiralite

This should be called difference instead of missing since it aligns with the mathematical terminology.

The difference method is exist and it's correct: see Maybe name it as complement?: see

Syjalo avatar Jul 29 '22 17:07 Syjalo

This should be called difference instead of missing since it aligns with the mathematical terminology.

The difference method is exist and it's correct: see Maybe name it as complement?: see

The diagram in your PR description showed a difference operation, which made me think the function was implemented as a difference operation.

As for the naming technically it's a "relative complement" because there's no concept of a universal set in our context. That being said, "complement" would be better because the lack of a universal set is implied.

suneettipirneni avatar Jul 29 '22 17:07 suneettipirneni

The problem here is that the existing difference method should be symmetricDifference, while this should be difference. But relativeComplement or similar is also valid; missing doesn't mean much. Also possibly valid is minus or subtract, since that's common notation for the set difference.

1Computer1 avatar Jul 29 '22 17:07 1Computer1

Codecov Report

Merging #8393 (6170c5f) into main (e74aa7f) will increase coverage by 0.02%. The diff coverage is 100.00%.

:exclamation: Current head 6170c5f differs from pull request most recent head 67eef90. Consider uploading reports for the commit 67eef90 to get more accurate results

@@            Coverage Diff             @@
##             main    #8393      +/-   ##
==========================================
+ Coverage   85.61%   85.63%   +0.02%     
==========================================
  Files          96       96              
  Lines        9459     9475      +16     
  Branches     1134     1137       +3     
==========================================
+ Hits         8098     8114      +16     
  Misses       1319     1319              
  Partials       42       42              
Flag Coverage Δ
brokers 65.24% <ø> (ø)
builders 98.65% <ø> (ø)
collection 100.00% <100.00%> (ø)
proxy 81.53% <ø> (ø)
rest 91.97% <ø> (ø)
util 100.00% <ø> (ø)
utilities 100.00% <ø> (ø)
voice 63.70% <ø> (ø)
ws 59.83% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/collection/src/collection.ts 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov[bot] avatar Jul 29 '22 17:07 codecov[bot]