HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcppMinor

Quicksort function

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
functionquicksortstackoverflow

Problem

I made a quicksort implementation based on this code. Please review my C++ version.

template
I partition(I low, I high) {
    auto p = *low;
    I i = low, j = high+1;
    while (true) {
        while (*(++i) = j) {
            break;
        }
        std::swap(*i, *j);
    }
    std::swap(*low, *j);
    return j;
}

template
void sort(C& c, I low, I high) {
    auto h = high == std::end(c) ? high-1 : high;
    if (h 
void sort(C& c) {
    if (c.size() == 0) {
        throw std::out_of_range("no size");
    }
    std::random_shuffle(std::begin(c), std::end(c));
    sort(c, std::begin(c), std::end(c));
}

Solution

This line is funky:

auto h = high == std::end(c) ? high-1 : high;


So on first call you are using one past the end. But on all other situations high is the actual end. The idiom in C++ is to use one past the end. So prefer to stick with standard conventions and use one past the end for high in all situations. This simplifies a lot of your code.

This is definitely wrong:

std::swap(*h, *(std::max_element(low, h)));


You are putting the largest element at the beginning. The result is that the partition does not work optimal (you basically are always getting the worst case scenario thus converting an O(n.ln(n)) algorithm into O(n^2) algorithm).
Minor things:

One variable per line.

I i = low;
    I j = high + 1;


When you partition the data put all values of the partition data on one side.

while (*(++i) < p) { }
    while (p < *(--j)) { }


What happens to data that equals p?

Code Snippets

auto h = high == std::end(c) ? high-1 : high;
std::swap(*h, *(std::max_element(low, h)));
I i = low;
    I j = high + 1;
while (*(++i) < p) { }
    while (p < *(--j)) { }

Context

StackExchange Code Review Q#75508, answer score: 2

Revisions (0)

No revisions yet.