Return to Answer

printVector

1. It should take the vector as a reference - there is no need to copy the entire vector just to print it.

2. You should make use of the newer features of the C++ language which were introduced since C++11, like auto and range-based for loops.

3. You can remove some conditional statements from the implementation by adjusting the logic slightly.

Overall the refactored function looks like this:

template<typename T>
void printVector(vector<T> &vect) {
 string separator = "";
 for (auto const &T item : vect) {
 cout << separator << item;
 separator = " ";
 }
 cout << endl;
}

swap

No need to re-invent the wheel. Use std::swap instead.

bubbleSort

1. You pass in the vector by value which means a copy of the original vector is passed in. This means the sort will not be visible outside of the function. So if you were to move printVector outside of bubbleSort you will see that it prints the unsorted original vector. You either need to pass the vector as a reference or return the sorted copy from the function.

2. int is not guaranteed to be able to index all items in a vector. You should use vector<T>::size_type for the index variable.

3. You can avoid the whole indexing by using iterators and iter_swap.

The refactored function looks like this:

void bubbleSort(vector<int> &numbers, long int iterations) {
 for(int i = 0; i < iterations; i++) {
 auto end = numbers.end() - 1;
 for (auto it = numbers.begin(); it != end; ++it)
 {
 if (*it > *(it + 1)) {
 std::iter_swap(it, it + 1);
 }
 }
 }
}

printVector

1. It should take the vector as a reference - there is no need to copy the entire vector just to print it.

2. You should make use of the newer features of the C++ language which were introduced since C++11, like auto and range-based for loops.

3. You can remove some conditional statements from the implementation by adjusting the logic slightly.

Overall the refactored function looks like this:

template<typename T>
void printVector(vector<T> &vect) {
 string separator = "";
 for (auto const &T item : vect) {
 cout << separator << item;
 separator = " ";
 }
 cout << endl;
}

swap

No need to re-invent the wheel. Use std::swap instead.

bubbleSort

1. You pass in the vector by value which means a copy of the original vector is passed in. This means the sort will not be visible outside of the function. So if you were to move printVector outside of bubbleSort you will see that it prints the unsorted original vector. You either need to pass the vector as a reference or return the sorted copy from the function.

2. int is not guaranteed to be able to index all items in a vector. You should use vector<T>::size_type for the index variable.

3. You can avoid the whole indexing by using iterators and iter_swap.

The refactored function looks like this:

void bubbleSort(vector<int> &numbers, long int iterations) {
 for(int i = 0; i < iterations; i++) {
 auto end = numbers.end() - 1;
 for (auto it = numbers.begin(); it != end; ++it)
 {
 if (*it > *(it + 1)) {
 std::iter_swap(it, it + 1);
 }
 }
 }
}