Rework Bubble Sort C++ #251
Conversation
june128
added
the
Implementation Edit
| #include <iostream> | ||
| #include <iterator> | ||
| #include <random> | ||
| #include <stdlib.h> // rand |
| } | ||
|
|
||
| for (;;) { | ||
| std::vector<int> bubble_sort(std::vector<int> unsorted_list) { |
There was a problem hiding this comment.
I would do sort in place
void bubble_sort(std::vector<int>& array)It will make code more performant (one array copy less) without any degradation of readability.
| auto input = generate_input(10, rng); | ||
| int main() | ||
| { | ||
| std::vector<int> to_be_sorted = {}; |
There was a problem hiding this comment.
style minor: = {} just a noise, do:
std::vector<int> to_be_sorted;| { | ||
| std::vector<int> to_be_sorted = {}; | ||
| // Initialize and print a vector with 50 random integers of domain [0,1000] | ||
| for (int i = 0; i < 50; i++) { |
There was a problem hiding this comment.
style minor: ++i is preferable style.
https://stackoverflow.com/questions/24853/what-is-the-difference-between-i-and-i
| std::vector<int> to_be_sorted = {}; | ||
| // Initialize and print a vector with 50 random integers of domain [0,1000] | ||
| for (int i = 0; i < 50; i++) { | ||
| int value = rand() % 1000; |
There was a problem hiding this comment.
You may want to initialize random generator with
srand(time(nullptr));|
|
||
| bubble_sort(begin(input), end(input)); | ||
| for (int i : sorted) { std::cout << i << " "; } |
There was a problem hiding this comment.
add
std::cout << std::endl; It is kind of annoying if terminal program does not do it.
|
|
||
| std::cout << "\nafter sorting:\n"; | ||
| print_range(std::cout, begin(input), end(input)); | ||
| return 0; |
There was a problem hiding this comment.
style minor: C++ does not require return 0
Really minor, I lot of people prefer return 0, for my taste it is just a noise.
| using std::swap; | ||
| swap(*it, *(it + 1)); | ||
| // Sweep through the array | ||
| for (unsigned int i = 0; i < unsorted_list.size() - 1; i++) { |
There was a problem hiding this comment.
style minor: ++i is preferable style.
https://stackoverflow.com/questions/24853/what-is-the-difference-between-i-and-i
| int main() | ||
| { | ||
| std::vector<int> to_be_sorted = {}; | ||
| // Initialize and print a vector with 50 random integers of domain [0,1000] |
There was a problem hiding this comment.
Comment is a lie, should be:
// Initialize and print a vector with 50 random integers of domain [0,999]
or
// Initialize and print a vector with 50 random integers of domain [0,1000)
First is preferable.
| { | ||
| std::vector<int> to_be_sorted = {}; | ||
| // Initialize and print a vector with 50 random integers of domain [0,1000] | ||
| for (int i = 0; i < 50; i++) { |
There was a problem hiding this comment.
I would provide a hand written test sample. Random test set usually does not cover edge cases and gives false confidence about the code.
In this case it does not cover cases with empty array, already sorted array and array with reverse order for instance.
|
I don't really have any criticism for your code, but I can tell you that we already have an open pull request for Bubble Sort in C++, which is #244. We can't possible merge both of them because they will certainly cause merge conflicts. Because we use a "first come - first serve" approach, I'm going to have to close this issue. I'm sorry for that, @benchislett. We still appreciate your contribution, though! I hope that you will stay around and find something else you can do for the Algorithm Archive. Just next time, make sure that there's not already a PR that does the same thing. ;) |
Bring on the criticism!