My QuickSort implementation skips the last value

Question

Relatively newbie programmer trying to implement a quicksort algorithm. The partition is always the first in each new array. I run pointers i and j from the left and right side until I find something bigger than the partition with i and smaller than the partition with j, then I swap both. The code works relatively fine, apart from the fact that it always skips the last value:

#include <iostream>
#include <string>
#include <iomanip>
#include <cstdlib>
#include <time.h>
#include <fstream>

using namespace std;
void QuickSort(int[], int, int);
void Partition(int[], int, int);
int main()
{
    int A[] = { 1, 3, 2, 6, 7, 5, 8, 9};
    QuickSort(A, 0, 7);
    for (int i = 0; i < 7; i++) {
        cout << A[i];
    }
}

void QuickSort(int A[], int first, int last) {
    if (last - first <= 1) {
        return;
    }

    Partition(A, first, last);



}

void Partition(int A[], int first, int last) {
    int partition = first;
    int i, j;
    i = first + 1;
    j = last;
    while (i < j) {
        if (A[i] < A[partition]) {
            while (A[i] < A[partition] && i != j) {
                i++;
            }
                if (A[i] >= A[partition]) {
                    while (A[j] > A[partition] && i != j) {
                        j--;

                    }

                }
                if (i == last && j == last && partition < i) {
                    if (A[partition] > A[i]) {
                        int k = A[partition];
                        int l = A[i];
                        A[i] = l;
                        A[j] = k;
                    }
                }
                int k, l;
                k = A[i];
                l = A[j];
                A[i] = l;
                A[j] = k;

        }
        else if (A[j] > A[partition]) {
            while (A[j] > A[partition] && i!=j) {
                j--;
            }
                if (A[j] < A[partition]) {
                    while (A[i] < A[partition] && i != j) {
                        i++;

                    }


                int k, l;
                k = A[i];
                l = A[j];
                A[i] = l;
                A[j] = k;
            }
        }

        else {
            int k, l;
            k = A[i];
            l = A[j];
            A[i] = l;
            A[j] = k;

        }

        i++;
        if (i > j) {
            i--;
        }

    }

    i = i - 1;
    int l, k;
    l = A[i];
    k = A[partition];
    A[i] = k;
    A[partition] = l;


    QuickSort(A, first, i);
    QuickSort(A, i + 1, last);
}

score 1 · Answer 1 · answered Jun 12 '20 at 07:03

1

It is just how you print result:

for (int i = 0; i < 7; i++) {
    cout << A[i];
}

It only prints first 7 elements in the array, while there are 8 elements in total. Try following instead

for (int i = 0; i < 8; i++) {
    cout << A[i];
}

It is a common off-by-1 error made by every programming learners, a good way to avoid this error is to remember always put i < array.size() in for loop condition.

answered Jun 12 '20 at 07:03

Kaiming Yang

41
3

I tried that but I still get errors running the program. And when I print with just 7, why does it skip over 5 when 5 is supposed to be dead center? – RogueNin7x Jun 12 '20 at 19:24
When applied my fix it seems running good and printing 12356789, as expected. What error do you get? – Kaiming Yang Jun 13 '20 at 00:31

My QuickSort implementation skips the last value

1 Answers1