Sorting vectors in a descending order using the quicksort algorithm

Question

I am new to quick sorting algorithms. Following the principle descried here Pivoting To Understand Quicksort. I try to write quicksort codes for sorting vector in a descending order. Here below is my codes with a simple example:

#include <iostream>
#include <vector>

using namespace std;

void quick_sort(vector<int> vec, int begin, int end)
{
    int i; //for left element index
    int j; //for right element index
    int pivot_idx = (begin+end)/2;
    int pivot = vec[pivot_idx]; //pivot element

    //cout<<"Pivot = "<<pivot<<endl;

    i = begin;
    j = end;
    //partition data into smaller and larger parts relative to the pivot
    while(1)
    {
       while(vec[i]>pivot)
       {
           ++i;
       }
       while(vec[j]<pivot)
       {
           ++j;
       }
       //Once the left index pass right index, partitioning is done
       if(i>=j)
       {
           break;
       }
       //otherwise do swapping
       int temp; 
       temp = vec[i];
       vec[i] = vec[j];
       vec[j] = temp;

       //keep moving indices
       i++;
       j--;
    }

    //recurse into two smaller parts
    if(begin<i-1)
    {
        quick_sort(vec, begin, i-1);
    }
    if(j+1<end)
    {
        quick_sort(vec, j+1, end);
    }
}

int main()
{  
    vector<int> v;
    v.push_back(9);
    v.push_back(5);
    v.push_back(2);
    v.push_back(6);
    v.push_back(1);
    v.push_back(11);
    v.push_back(3);

    cout<<"Before sorting:"<<endl;
    for(int i=0;i<v.size();i++)
    {
        cout<<v[i]<<endl;
    }

    quick_sort(v,0,v.size()-1);
    cout<<"After sorting:"<<endl;
    for(int i=0;i<v.size();i++)
    {
        cout<<v[i]<<endl;
    }

    return 0;
}

The codes above do not work as expected. No sorting is performed. I need some help to debug my codes to found out why. Thanks!

Why not simply change the comparison code when comparing two items? Why are you attempting to rewrite the algorithm just to sort in a descending order? Changing an algorithm just because the sort criteria changes is not the way you write sort algorithms. — PaulMcKenzie, Feb 17 '19 at 02:53
[Here is an example](https://stackoverflow.com/questions/24650626/how-to-implement-classic-sorting-algorithms-in-modern-c). Look at the quicksort sample. The very same code works whether the sort is ascending, descending, etc, by providing a comparer function. The only thing that needs to change is the question "does item A come before item B?" Nothing more, nothing less. In other words, your code should just have a function asking that question, and then whatever is returned, `true` or `false`, the code takes that result and uses that information. No need to change anything else. — PaulMcKenzie, Feb 17 '19 at 02:55
Looks like a Hoare partition. Typically the `i++` is inside a `do`/`while` so that the test is after the increment. The `j++` should be a `j--` and also be in a `do`/`while`. Looks like you figured most of that out on your own. See if switching to a `do`/`while` speeds that up a bit. — user4581301, Feb 17 '19 at 03:02
You should not name a variable `end` if it point to `last` element as it will confuse other people as `end` usually point past last element. — Phil1970, Feb 17 '19 at 03:11
Given that you take inspiration from the quoted link, then you should implement the code so that it follows exactly what it is described in the article. Once that is done and properly tested, make appropriate changes to sort in decreasing order. **You don't follow items 5/ and 6/ of the guide**. — Phil1970, Feb 17 '19 at 03:22
First write code for sorting in ascending order and **use same sample data** as in the article you reference. Then **use a debugger** and follow the code **step by step** and ensure that it and get the **same result at each step** as explained in the example. Once that works properly, **generalize the comparison** to reverse sorting order. — Phil1970, Feb 17 '19 at 03:27

score 4 · Accepted Answer · answered Feb 17 '19 at 02:38

4

You are passing the vector by value, and so you sort a copy of the original vector. Pass it by reference instead, that is change the signature to

void quick_sort(vector<int>& vec, int begin, int end)
                          ^^^

answered Feb 17 '19 at 02:38

Anton Savin

40,838
8
54
90

I have changed the function signature, but sorting still cannot be achieved. – jwm Feb 17 '19 at 02:43
@jingweimo never assume you only have one bug, and don't update a question in such a way that you render a correct answer incorrect. – user4581301 Feb 17 '19 at 02:50

score 4 · Answer 2 · answered Feb 17 '19 at 03:02

4

There is a mistake in updating j index:

while(vec[j]<pivot){++j;}

should be

while(vec[j]<pivot){--j;}

answered Feb 17 '19 at 03:02

jwm

4,832
10
46
78

score 1 · Answer 3 · answered Feb 17 '19 at 03:50

There were a few other bugs than just passing by reference that I fixed mainly by making this implementation more simple. Your function was trying to do everything and it wasn't giving correct results. I split the partition and the swap into their own functions and used a very standard quicksort function. This seems to work find now:

#include <iostream>
#include <vector>
using namespace std;


void swap(int *a, int *b) {
    int temp = *a;
    *a = *b;
    *b = temp;
}


int partition (vector<int> &vec, int l, int h) {
    int x = vec[h];
    int i = (l - 1);

    for (int j = l; j <= h- 1; j++) {
        if (vec[j] <= x) {
            i++;
            swap (&vec[i], &vec[j]);
        }
    }
    swap (&vec[i + 1], &vec[h]);
    return (i + 1);
}


void quick_sort(vector<int> &vec, int begin, int end) {
    if (begin < end) {
        int pivot_idx = partition(vec, begin, end);
        quick_sort(vec, begin, pivot_idx-1);
        quick_sort(vec, pivot_idx+1, end);
    }
}


int main() {
    vector<int> v;
    v.push_back(9);
    v.push_back(5);
    v.push_back(2);
    v.push_back(6);
    v.push_back(1);
    v.push_back(11);
    v.push_back(3);

    cout<<"Before sorting:"<<endl;
    for(int i=0;i<v.size();i++) {
        cout<<v[i]<<endl;
    }

    quick_sort(v,0,v.size()-1);
    cout<<"After sorting:"<<endl;
    for(int i=0;i<v.size();i++) {
        cout<<v[i]<<endl;
    }

    return 0;
}

I hope this helps

I realize this sorts in ascending order, it won't take much to reverse a sorted vector or convert this sort to descending depending on what you prefer. — Vic, Feb 17 '19 at 03:55

Sorting vectors in a descending order using the quicksort algorithm

3 Answers3