Hoare's partition not working correctly (quicksort)

Question

So after following the quicksort and hoares partition algorithm from Cormen, this is the code that I was able to produce. The array comes out partly sorted with uninitialized elements/garbage elements and I can't for the life of me figure out why... I thought I followed the algorithm exactly as the book writes it.

Here is the pseudocode straight from the book:

HOARE-PARTITION(A, p, r)
1 x = A[p]
2 i = p - 1
3 j = r + 1
4 while TRUE
5     repeat
6        j = j - 1
7     until A[j] <= x
8     repeat
9        i = i + 1
10    until A[i] >= x
11    if i < j
12        exchange A[i] with A[j]
13    else return j

And here is the C++ code I translated it into:

void quickSort(int arr[], int p, int r){
    if(p < r){
        int q = hoare_partition(arr, p, r);
        quickSort(arr, p, q-1);
        quickSort(arr, q+1, r);
    }
}

int hoare_partition(int arr[], int p, int r){
    int x = arr[p];
    int i = p - 1;
    int j = r + 1;
    while(true){
        do{
            j--;
        }while(arr[j] > x);
        do{
            i++;
        }while(arr[i] < x);
        if(i < j){
            int temp = arr[i];
            arr[i] = arr[j];
            arr[j] = temp;
        }
        else 
            return j;
    }
}

Im using the following to test it

cout << endl << endl << "Testing quicksort" << endl;
int tarr[10] = {2, 30, 1, 99, 46, 33, 48, 67, 23, 76};
quickSort(tarr, 0, 10);
cout << "arr after quicksort: ";
for(int i = 0; i < 10; i++){
    cout <<  tarr[i] << " ";
}
cout << endl;

The output

arr after quicksort: -2146162183 1 2 23 30 33 46 48 67 76

Any help is appreciated ahead of time...thanks

EDIT

Changing the test case call to quickSort(arr, 0, 9) fixed it for this situation.

However, with a reverse sorted array as the input this is the output:

arr2 is:
30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1
arr2 after quicksort :
1 3 5 7 9 11 13 15 17 19 20 21 18 22 16 23 14 24 12 25 10 26 8 27 6 28 4 29 2 30

using this test code:

int arr2[30];
fillArrayReverse(arr2, 30);
cout << "arr2 is :" << endl;
for(int i = 0; i < 30; i++){
    cout << arr2[i] << " ";
}
cout << endl;
quickSort(arr2, 0, 29);
cout << "arr2 after quicksort: " << endl;
for(int i = 0; i < 30; i++){
    cout << arr2[i] << " ";
}
cout << endl;

You do need to watch for 0-based arrays as opposed to 1-based arrays, as you note in a comment to the answer. — Jonathan Leffler, Mar 13 '15 at 05:46

MildWolfie · Answer 1 · 2020-01-07T18:50:55.813

6

The psuedocode in the book is correct. The issue is not that j was returned (this was correct), but in the quickSort function itself. It should actually read as follows:

void quickSort( int arr[], int p, int r )
{
    if( p < r )
    {
        int q = hoare_partition( arr, p, r );
        quickSort( arr, p, q );
        quickSort( arr, q + 1, r );
    }
}

Note that we remove q - 1 in the first call to quickSort, and replace it with just q. Beforehand we were essentially skipping the pivot point and only sorting items around it. I noticed this discrepancy after taking a look at the Hoare Partition psuedocode on quick sort on wikipedia.

edited Jan 07 '20 at 18:50

answered Feb 09 '16 at 16:46

MildWolfie

2,082
1
14
27

2

Thank you for posting this, the other answer is clearly wrong by stating you should return i instead of j. The point of Hoare's is not to separate the array into `[items smaller than partition] partitioning element [items larger than or equal to partition]`, it's to separate the array into `[items smaller] [items larger or equal]`. As you mentioned, we then quicksort the two subarrays - but unlike the Lomuto partition, the partitioning element is not already in the correct place so it must be included in the recursive calls to quicksort. – FCStrike Apr 11 '16 at 16:00

score 3 · Accepted Answer · edited May 23 '17 at 10:29

3

It's a problem with your test code. The third parameter should be the index of the last element, not the number of elements:

quickSort(tarr, 0, 9);

You're reading past the end of the array.

For more information about Hoare partitioning, Quicksort and Cormen etc, see this question.

The simple fix for the other problems you're having is to change hoare_partition() to return i instead of j. See the link for more details (it's an error in the book reportedly).

edited May 23 '17 at 10:29

Community

1
1

answered Mar 13 '15 at 05:19

samgak

22,290
4
50
73

In the book it says "To sort an entire array A, the initial call is QUICKSORT(A, 1, A.length)." But then again the book goes by 1-indexed arrays. So that must be why it's using the length as opposed to length-1.... – JayB Mar 13 '15 at 05:21
It still doesn't work on a reverse sorted array, made an edit to my question with details – JayB Mar 13 '15 at 05:34

Hoare's partition not working correctly (quicksort)

EDIT

2 Answers2

Linked