4
\$\begingroup\$

I have implemented insertion sort using C++. Is there any way to improve my code and is it CppCoreGuideline compliant? As I passed the vector as const in print method, do I need to specify const in range for loop?

/*
 * Insertion Sort
 */
#include <iostream>
#include <vector>
void insertionSort(std::vector<int> &v);
void print(const std::vector<int> &v);
int main()
{
 std::ios_base::sync_with_stdio(false);
 std::vector<int> v {10, 9, 8, 7, 6, 5, 4, 3, 2, 1};
 // Print the vector before sorting.
 print(v);
 // Sort
 insertionSort(v);
 // Print the vector after sorting.
 print(v);
 return 0;
}
void insertionSort(std::vector<int> &v) {
 for (auto i = v.begin() + 1; i != v.end(); ++i) {
 int key = *i;
 auto j = i - 1;
 while (j >= v.begin() && *j > key) {
 *(j+1) = *j;
 --j;
 }
 *(j+1) = key;
 }
 return;
}
void print(const std::vector<int> &v) {
 for (const auto &i : v) {
 std::cout << i << " ";
 }
 std::cout << "\n";
}
asked Jan 15, 2017 at 10:16
\$\endgroup\$

3 Answers 3

5
\$\begingroup\$

Advice 1

In your insertionSort you can remove the last statement (return).

Advice 2

Making your insertion sort generic is easy:

template<typename RandIt>
void insertion_sort(RandIt begin, RandIt end)
{
 for (auto i = begin + 1; i != end; ++i)
 {
 auto key = *i;
 auto j = i - 1;
 while (j >= begin and *j > key)
 {
 *(j + 1) = *j;
 --j;
 }
 *(j + 1) = key;
 }
}
template<typename Container>
void insertion_sort(Container& cont)
{
 insertion_sort(std::begin(cont), std::end(cont));
}

Advice 3

Making your print function generic is not hard either:

template<typename RandIt>
void print(RandIt begin, RandIt end)
{
 using value_type = typename std::iterator_traits<RandIt>::value_type;
 std::copy(begin, end, std::ostream_iterator<value_type>(std::cout, ", "));
 std::cout << std::endl;
}
template<typename Container>
void print(const Container& container)
{
 print(std::begin(container), std::end(container));
}

Hope that helps.

answered Jan 15, 2017 at 10:54
\$\endgroup\$
1
\$\begingroup\$

A few things to think about:

1. Sorting in terms of iterators, and allowing a customisable predicate is more 'standard-like':

template<class Iter, class Pred = std::less<> >
void insertionSort(Iter first, Iter last, Pred pred = Pred())
{
 for (auto i = first; i != last; i = std::next(i)) {
 std::rotate(std::upper_bound(first, i, *i, pred), i, std::next(i));
 }
}

2. Why not also provide a range-like interface?:

template<class Container, class Pred = std::less<>>
Container& insertionSort(Container&& cont, Pred pred = Pred())
{
 insertionSort(std::begin(cont), std::end(cont), std::forward<Pred>(pred));
 return cont;
}

3. Why not provide an iomanip-like function to provide printing of vectors in a structured way:

template<class T, class A>
struct vector_printer
{
 std::ostream& operator()(std::ostream& os) const {
 const char* sep = "";
 const char* pre_brace = "";
 os << "[";
 for (auto&& e : vec_) {
 os << sep << e;
 sep = ", ";
 pre_brace = " ";
 }
 return os << pre_brace << "]";
 }
 const std::vector<T, A>& vec_;
};
template<class T, class A>
decltype(auto) operator<<(std::ostream& os, vector_printer<T, A> const& p)
{
 return p(os);
}
template<class T, class A>
auto print(std::vector<T, A> const& v)
{
 return vector_printer<T, A> { v };
}

This then allows us to write expressive code:

int main()
{
 std::ios_base::sync_with_stdio(false);
 std::vector<int> v {10, 9, 8, 7, 6, 5, 4, 3, 2, 1};
 // Print the vector before sorting.
 std::cout << print(v) << std::endl;
 // Print the vector after sorting.
 std::cout << print(insertionSort(v)) << std::endl;
 // ascending order
 std::cout << print(insertionSort(v, std::greater<>())) << std::endl;
 return 0;
}

Complete code:

#include <iostream>
#include <vector>
#include <algorithm>
#include <string>
template<class Iter, class Pred = std::less<> >
void insertionSort(Iter first, Iter last, Pred pred = Pred())
{
 for (auto i = first; i != last; i = std::next(i)) {
 std::rotate(std::upper_bound(first, i, *i, pred), i, std::next(i));
 }
}
template<class Container, class Pred = std::less<>>
Container& insertionSort(Container&& cont, Pred pred = Pred())
{
 insertionSort(std::begin(cont), std::end(cont), std::forward<Pred>(pred));
 return cont;
}
template<class T, class A>
struct vector_printer
{
 std::ostream& operator()(std::ostream& os) const {
 const char* sep = "";
 const char* pre_brace = "";
 os << "[";
 for (auto&& e : vec_) {
 os << sep << e;
 sep = ", ";
 pre_brace = " ";
 }
 return os << pre_brace << "]";
 }
 const std::vector<T, A>& vec_;
};
template<class T, class A>
decltype(auto) operator<<(std::ostream& os, vector_printer<T, A> const& p)
{
 return p(os);
}
template<class T, class A>
auto print(std::vector<T, A> const& v)
{
 return vector_printer<T, A> { v };
}
int main()
{
 std::ios_base::sync_with_stdio(false);
 std::vector<int> v {10, 9, 8, 7, 6, 5, 4, 3, 2, 1};
 // Print the vector before sorting.
 std::cout << print(v) << std::endl;
 // Sort
 // Print the vector after sorting.
 std::cout << print(insertionSort(v)) << std::endl;
 // ascending order
 std::cout << print(insertionSort(v, std::greater<>())) << std::endl;
 // sort now works on any container:
 std::cout << insertionSort(std::string("ZXCVBNMASDFGHJKLQWERTYUIOP")) << std::endl;
 return 0;
}

Expected output:

[10, 9, 8, 7, 6, 5, 4, 3, 2, 1 ]
[1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ]
[10, 9, 8, 7, 6, 5, 4, 3, 2, 1 ]
ABCDEFGHIJKLMNOPQRSTUVWXYZ
answered Jan 15, 2017 at 15:44
\$\endgroup\$
-2
\$\begingroup\$

Your code looks fine, but it has a run time complexity of O(n*n), you can perform this in logarithmic time:

std::vector<int> to_insert_sort {2,3,5,6,1,1,3,2};
int to_insert {3};
std::sort(to_insert_sort.begin(), to_insert_sort.end()); //Log
auto pos = std::upper_bound(to_insert_sort.begin(), to_insert_sort.end(), 5); //Log
to_insert_sort.insert(pos, to_insert); //O(n)
answered Jan 15, 2017 at 10:33
\$\endgroup\$
2
  • 1
    \$\begingroup\$ std::sort is going to be O(n*log n), so still an order of magnitude better than the original O(n^2), but not nearly as good as the advertised logarithmic. :-) \$\endgroup\$ Commented Jan 15, 2017 at 17:59
  • 3
    \$\begingroup\$ Insertion sort is \$\Theta(n^2)\$ in average and worst cases. \$\endgroup\$ Commented Jan 15, 2017 at 18:35

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.