-
Notifications
You must be signed in to change notification settings - Fork 19.9k
refactor: cleanup CountingSort
#5275
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5275 +/- ##
============================================
+ Coverage 40.34% 40.52% +0.18%
- Complexity 2486 2500 +14
============================================
Files 519 521 +2
Lines 15479 15483 +4
Branches 2950 2954 +4
============================================
+ Hits 6245 6275 +30
+ Misses 8944 8916 -28
- Partials 290 292 +2 ☔ View full report in Codecov by Sentry. |
… not counting sort
Hello @vil02 . Could you please advise on how to handle the Pseudo Counting Sort classes? To preserve the integrity and original purpose of this repository, it might be best to remove them since they do not actually implement Counting Sort. However, they have been part of the repository for some time, and I'm unsure of the best course of action. |
@alxkm: I would suggest to remove them. I would guess, the original implementation was merged by accident. Introducing some Therefore I suggest to:
In any case: thanks to you we have spotted a quite serious bug! |
Ok, thank you. Got it. This is really proper way to resolve this issue |
This implementation of CountingSort is wrong. Because it works only using TreeMap, which causes wrong asymptotic for CountingSort. This issue will be resolved in such way:
All these discussed in this PR |
Cleanup CountingSort
Adding test for it
Extracted method CountingSort using stream into separate class also added test for it
clang-format -i --style=file path/to/your/file.java