Skip to main content
added 1 character in body
Source Link
Morwenn
  • 20.2k
  • 3
  • 69
  • 132

Moreover, most of the sorting algorithms in the standard library that deal with algorithms additionally accept an optional Compare parameter so that you usecan use something else than operator< to compare two elements. Assuming you have a C++14 compiler, you can use std::less<> as a default:

Now, letslet's have a look at the functions that you have reimplemented and that you can already find in the standard library (there is some overlap with other answers):

So, taking all of that into account, here is how a modern C++ mergesort shouldcould look:

This is better, bubut still not perfect: the agorithmalgorithm currently works with random-access iterators, which is ok with std::vector, std::deque or std::array, but it also means that it doesn't work with std::list or std::forward_list which respectively expose bidirectional iterators and forward iterators. std::inplace_merge doesn't work with forward iterators (yet?) but it works fine with bidirectional iterators; here is what we have to change to make our mergeSort work with bidirectional iterators:

  • Change iterators subtractions by a callcalls to std::distance.
  • Change the iterator-size addition by std::next.
  • Change the name of the template parameters so that they do not lie.

Moreover, most of the standard library that deal with algorithms additionally accept an optional Compare parameter so that you use use something else than operator< to compare two elements. Assuming you have a C++14 compiler, you can use std::less<> as a default:

Now, lets have a look at the functions that you have reimplemented and that you can already find in the standard library (there is some overlap with other answers):

So, taking all of that into account, here is how a modern C++ mergesort should look:

This is better, bu still not perfect: the agorithm currently works with random-access iterators, which is ok with std::vector, std::deque or std::array, but it also means that it doesn't work with std::list or std::forward_list which respectively expose bidirectional iterators and forward iterators. std::inplace_merge doesn't work with forward iterators (yet?) but it works fine with bidirectional iterators; here is what we have to change to make our mergeSort work with bidirectional iterators:

  • Change iterators subtractions by a call to std::distance.
  • Change the iterator-size addition by std::next.
  • Change the name of the template parameters so that they do not lie.

Moreover, most of the sorting algorithms in the standard library additionally accept an optional Compare parameter so that you can use something else than operator< to compare two elements. Assuming you have a C++14 compiler, you can use std::less<> as a default:

Now, let's have a look at the functions that you have reimplemented and that you can already find in the standard library (there is some overlap with other answers):

So, taking all of that into account, here is how a modern C++ mergesort could look:

This is better, but still not perfect: the algorithm currently works with random-access iterators, which is ok with std::vector, std::deque or std::array, but it also means that it doesn't work with std::list or std::forward_list which respectively expose bidirectional iterators and forward iterators. std::inplace_merge doesn't work with forward iterators (yet?) but it works fine with bidirectional iterators; here is what we have to change to make our mergeSort work with bidirectional iterators:

  • Change iterators subtractions by calls to std::distance.
  • Change the iterator-size addition by std::next.
  • Change the name of the template parameters so that they do not lie.
Source Link
Morwenn
  • 20.2k
  • 3
  • 69
  • 132

This answer is mostly about design and good practice, and not so much about the algorithms themselves. Therefore, I will mostly use mergeSort in the examples.

For the sake of completeness: a sorting algorithm with an array + size interface is really C-ish. A proper C++ algorithm would take a pair of iterators and would be templated:

template<typename RandomAccessIterator>
void mergeSort(RandomAccessIterator first, RandomAccessIterator last);

Moreover, most of the standard library that deal with algorithms additionally accept an optional Compare parameter so that you use use something else than operator< to compare two elements. Assuming you have a C++14 compiler, you can use std::less<> as a default:

template<
    typename RandomAccessIterator,
    typename Compare = std::less<>
>
void mergeSort(RandomAccessIterator first, RandomAccessIterator last,
               Compare compare={});

Now, lets have a look at the functions that you have reimplemented and that you can already find in the standard library (there is some overlap with other answers):

  • swapElements actually performs std::swap(toSort[i], toSort[j]);, or std::swap(*it1, *it2); if we consider that it1 and it2 are iterators corresponding to toSort + i and toSort + j. Note that we have templated mergeSort so that it can sort any random-access collection of any type. We would like our program to be able to use user-defined swap function instead of std::swap so that it can take advantage of the potential optimizations. We can use argument-dependent lookup to do the job:

      using std::swap;
      swap(*it1, *it2);
    

    The first line tells the compiler to take std::swap into account for unqualified calls to swap. The second makes an unqualified call to swap: the compiler will check the types of *it1 and *it2 and search for a swap function in their associated namespace. If the lookup finds such a function, it will use it, otherwise it will use std::swap instead. Note that you can also use std::iter_swap which does exactly that:

      std::iter_swap(it1, it2);
    
  • partitionElements could probably be replaced by std::partition.

  • You can replace copyArray by std::copy, or std::move if you know that you won't read the elements after they have been moved-from. Note that std::sort from the standard library is required to work with move-only types too, so it would be nice if you could ensure that your sorting algorithms work with such types (agreed, it's not trivial for quicksort).

  • I am pretty sure that mergeParts could be replaced by std::inplace_merge too.


So, taking all of that into account, here is how a modern C++ mergesort should look:

template<typename RandomAccessIterator, typename Compare>
void mergeSort(RandomAccessIterator first, RandomAccessIterator last,
               Compare compare, std::size_t size) {
    if (size < 2) return;
    auto middle = first + size / 2;
    mergeSort(first, middle, compare, size / 2);
    mergeSort(middle, last, compare, size - size/2);
    std::inplace_merge(first, middle, last, compare);
}

template<
    typename RandomAccessIterator,
    typename Compare = std::less<>
>
void mergeSortImpl(RandomAccessIterator first, RandomAccessIterator last,
               Compare compare={})
{
    std::size_t size = last - first;
    mergeSortImpl(first, last, compare, size);
}

This is better, bu still not perfect: the agorithm currently works with random-access iterators, which is ok with std::vector, std::deque or std::array, but it also means that it doesn't work with std::list or std::forward_list which respectively expose bidirectional iterators and forward iterators. std::inplace_merge doesn't work with forward iterators (yet?) but it works fine with bidirectional iterators; here is what we have to change to make our mergeSort work with bidirectional iterators:

  • Change iterators subtractions by a call to std::distance.
  • Change the iterator-size addition by std::next.
  • Change the name of the template parameters so that they do not lie.

Those simple functions from the header <iterator> work with any category of iterator that is at least forward iterator. That also means that if one day std::inplace_merge works with forward iterators, then mergeSort will also work with forward iterators out-of-the-box and allow to sort, for example, singly linked lists. Here is our new enhanced algorithm:

template<typename RandomAccessIterator, typename Compare>
void mergeSort(RandomAccessIterator first, RandomAccessIterator last,
               Compare compare, std::size_t size) {
    if (size < 2) return;
    auto middle = std::next(first, size / 2);
    mergeSort(first, middle, compare, size / 2);
    mergeSort(middle, last, compare, size - size/2);
    std::inplace_merge(first, middle, last, compare);
}

template<
    typename RandomAccessIterator,
    typename Compare = std::less<>
>
void mergeSortImpl(RandomAccessIterator first, RandomAccessIterator last,
               Compare compare={})
{
    std::size_t size = std::distance(first, last);
    mergeSortImpl(first, last, compare, size);
}

More than an in-depth review of your algorithms, this answer was more about good pratices when writing algorithms. Basically, here is what you should keep in mind:

  • Make your algorithms generic when possible.
  • Iterators are the way to go (even though ranges will enhance things).
  • The standard library can help you.
  • Categories of iterators matter.
  • Custom comparisons are cool (if you give std::greater<> instead of std::less<> to a sorting algorithm, it will sort the collection in reverse order).