8000 Fix `Permutations` for when source length is >12 by atifaziz · Pull Request #979 · morelinq/MoreLINQ · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix Permutations for when source length is >12 #979

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

Merged
merged 1 commit into from
Mar 8, 2023

Conversation

atifaziz
Copy link
Member
@atifaziz atifaziz commented Mar 2, 2023

This PR fixes #978, by using ulong internally instead of int and adding checked arithmetic. This limits to 18,446,744,073,709,551,615 permutations and will overflow thereafter. A document comment has also been added to call out this limitation.

Due to the computationally expensive nature of enumerating permutations of a source with more than 12 elements, no unit test has been added to exercise the fix. However, a LINQPad query was used as an eyeball test: MoreLinqPermutations.zip. At the time of opening this PR, it will use the latest NuGet package version (3.4.0) and exercise the bug. If the package reference is changed to an assembly reference that's the output of this PR, then it will exercise the fix.

@atifaziz atifaziz changed the title Fix Permutations for source length > 12 Fix Permutations for when source length is >12 Mar 2, 2023
@codecov
Copy link
codecov bot commented Mar 2, 2023

Codecov Report

Merging #979 (ab9c85b) into master (b99a6a8) will not change coverage.
The diff coverage is 100.00%.

❗ Current head ab9c85b differs from pull request most recent head 20d6798. Consider uploading reports for the commit 20d6798 to get more accurate results

@@           Coverage Diff           @@
##           master     #979   +/-   ##
=======================================
  Coverage   92.60%   92.60%           
=======================================
  Files         113      113           
  Lines        3434     3434           
  Branches     1052     1054    +2     
=======================================
  Hits         3180     3180           
  Misses        191      191           
  Partials       63       63           
Impacted Files Coverage Δ
MoreLinq/NestedLoops.cs 100.00% <100.00%> (ø)
MoreLinq/Permutations.cs 96.42% <100.00%> (ø)

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

@atifaziz atifaziz merged commit e1c767f into morelinq:master Mar 8, 2023
@atifaziz atifaziz deleted the fix/permutations branch March 8, 2023 22:33
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.

Permutations returns wrong number of permutations for N > 12
2 participants
0