-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
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
tests: Add test in cycle_sort.cpp
#1520
Conversation
@Panquesito7 Sorry for taking your time in the same kind of file edit/PR again. Its because I contributed for the first time and just to get updates from the master repo to forked repo I forked it again. But I assure you that it won't take long for this PR since you already approved it and I have used the same code as before basically re-created the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the only thing missing! After that, I'll approve this PR. 🙂
sorting/cycle_sort.cpp
Outdated
* @brief Implementation of [Cycle | ||
* sort](https://en.wikipedia.org/wiki/Cycle_sort) algorithm | ||
* | ||
* @brief Implementation of [Cycle sort](https://en.wikipedia.org/wiki/Cycle_sort) algorithm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like you to put this back as it was (same in lines 5 and 21).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are currently 1-line descriptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference
Do this contribution is some different. We already have a cycle_sort. @Panquesito7
https://github.com/TheAlgorithms/C-Plus-Plus/projects/6#card-63413841 |
Co-authored-by: Abhinn Mishra <49574460+mishraabhinn@users.noreply.github.com>
Co-authored-by: Abhinn Mishra <49574460+mishraabhinn@users.noreply.github.com>
Hey, after committing the above suggestions I got mail as Run failed: CodeQL... but here everything is working fine. Isn't it? |
Is it working now? |
Yea, I think so. 😕 |
If you got mail there is issue. Something went wrong. Go to workflow and see the error. |
sorting/cycle_sort.cpp
Outdated
@@ -98,9 +98,9 @@ static void test() { | |||
std::cout << "passed" << std::endl; | |||
|
|||
// [4.3, -6.5, -7.4, 0, 2.7, 1.8] return [-7.4, -6.5, 0, 1.8, 2.7, 4.3] | |||
std::vector<double> array2 = {4.3, -6.5, -7.4, 0, 2.7, 1.8}; | |||
std::vector<int> array2 = {4.3, -6.5, -7.4, 0, 2.7, 1.8}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why int ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry 1 sec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20 minutes and no mail yet 😕
@mishraabhinn errors resolved 😃 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there! 😄
Co-authored-by: David Leal <halfpacho@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thank you for your contribution! 👍 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Earlier approval by mistake.
test.cpp: In instantiation of ‘std::vector<T> sorting::cycle_sort::cycleSort(const std::vector<T>&) [with T = unsigned int]’:
test.cpp:96:64: required from here
test.cpp:53:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
53 | if (pos == cycle_start) {
| ~~~~^~~~~~~~~~~~~~
test.cpp:59:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
59 | if (pos == cycle_start) {
| ~~~~^~~~~~~~~~~~~~
test.cpp:65:20: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
65 | while (pos != cycle_start) {
| ~~~~^~~~~~~~~~~~~~
test.cpp: In instantiation of ‘std::vector<T> sorting::cycle_sort::cycleSort(const std::vector<T>&) [with T = double]’:
test.cpp:103:62: required from here
test.cpp:53:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
53 | if (pos == cycle_start) {
| ~~~~^~~~~~~~~~~~~~
test.cpp:59:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
59 | if (pos == cycle_start) {
| ~~~~^~~~~~~~~~~~~~
test.cpp:65:20: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
65 | while (pos != cycle_start) {
| ~~~~^~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
shell returned 1
Sorry for this have changed the data type of iterators. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All fine for me. I'll merge this now. If anybody finds any objections, please make another PR to fix them. Thanks! 😄
Description of Change
Checklist
Notes: