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

Quicksort in C++

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

Problem

Here is the implementation I ended up with. Please post your feedback!

#include 
#include 

#include 

inline void swap(int * const a, const int i, const int j) {

    const int tmp = a[i];
    a[i] = a[j];
    a[j] = tmp;
}

void printArray(int *a, int len) {
    for(int i=0; i pivot)
            upper--;

        printArray(a, len);

        if(lower == upper)
            break;

        swap(a, lower, upper);
    }

    printf("First Part ");
    printArray(a, lower);
    printf("Second Part ");
    printArray(a + lower + 1, len - lower - 1);
    printf("\n");

    qsort(a, lower);
    qsort(a + lower + 1, len - lower - 1);
}

int main() {

    int a[] = {1, 7, 3, 6, 5, 9, 2, 0, 4, 8};
    int len = sizeof(a)/sizeof(int);

    qsort(a, len);
    printArray(a, len);

    std::cin.get();
}

Solution

#include 
#include 

#include 


If you're writing C++ prefer cout and cin provided by iostream. They provide much better type-safety compared to their C counterparts. OTOH, you can still use printf and scanf if you don't need the extra flexibility provided by C++ streams and you want to keep overhead to a minimal. At any rate, it doesn't make sense to include both headers here -- you're either using one or the other.

inline void swap(int *const a, const int i, const int j) 
// ...


You don't need to write your own swap. The standard library provides a fully working std::swap which you can call like this:

std::swap(a[lower], a[upper]);


Note that const isn't really adding anything here for your qsort prototype since everything's pass-by-value. A subtle point is that pointer a below is also pass-by-value. Changing a in qsort won't affect where the original 'a' pointers to(the one passed by main).

void qsort(int * const a, const int len)
// ...


This qsort signature is more reminiscence of a C-style function than C++. In C++ it's more common to accept a range to sort by templatizing the parameters with iterator types. Something more akin to this for example:

template 
void qsort(RANDOMIT begin, const RANDOMIT &end);


This is well covered in other quicksort questions already. Take a look here and here for a more detailed explanation and other ideas you can explore.

I would also factor out the partitioning code to its own function just to keep the tasks well-defined.

while(true) {
    while(a[lower]  pivot)
        upper--;

    printArray(a, len);

    if(lower == upper)
        break;

    swap(a, lower, upper);
}


There's also a bug lurking above -- duplicate items will cause an infinite loop. A possible fix might be:

while(true) 
{
    // ...
    if(lower >= upper) break;

    if(a[lower] != a[upper]) { std::swap(a[lower], a[upper]); }
    ++lower, --upper;
}

Code Snippets

#include <cstdio>
#include <cstdlib>

#include <iostream>
inline void swap(int *const a, const int i, const int j) 
// ...
std::swap(a[lower], a[upper]);
void qsort(int * const a, const int len)
// ...
template <class RANDOMIT>
void qsort(RANDOMIT begin, const RANDOMIT &end);

Context

StackExchange Code Review Q#6751, answer score: 11

Revisions (0)

No revisions yet.