0

Short version of original question: I am trying to convert this example: http://programmertech.com/program/cpp/quick-sort-recursive-and-iterative-in-c-plus-plus of iterative quicksort to use vectors rather than arrays and begin simplifying a few things. Originally the direct conversion failed, so I tried through it line by line to get a better understanding, but my logic is stuck and broken.

EDIT: Deleted everything from the question, providing a minimal (not quite working) example of what I'm attempting. This is everything I have so far. I have worked through this on paper by hand, and the more I've touched it the worse it gets, and now gets stuck in an infinite loop (originally wasn't sorting correctly).

Here is my thinking: getMedian as I've written it should swap the pivot value, and the left and right values so that they are ordered: left <= med <= right. When we go to the while (right > left) loop in the partition algorithm, it should keep swapping elements to put all of those greater than the pivot to the right of it, and those less to the left. The stack keeps a track of the Sub(vectors) (in this case) which need to be partitioned still. But that doesn't seem to be working. I feel as if I've missed something very important to this working.

#include <iostream>
#include <vector>

class QuickSort {
public:
    QuickSort(std::vector<int> toBeSorted) : toBeSorted(toBeSorted) {}
    void sortVector();
    void print();
private:
    int partition(int left, int right);
    int getMedian(int left, int right);

    std::vector<int> toBeSorted;
};

// Iterative method using a stack
void QuickSort::sortVector() {
    int stack[toBeSorted.size()];
    int top = 0;
    stack[top++] = toBeSorted.size() - 1;
    stack[top++] = 0;

    int left, right, pivIndex;

    while (top > 0) {
        // Popping values for subarray
        left = stack[--top];
        right = stack[--top];
        pivIndex = partition(left, right);

        if (pivIndex + 1 < right) {
            stack[top++] = right;
            stack[top++] = pivIndex+1;
        }

        if (pivIndex - 1 > left) {
            stack[top++] = pivIndex-1;
            stack[top++] = left;
        }
    }
}

int QuickSort::partition(int left, int right) {
    int pivotValue = getMedian(left, right);

    if (right - left > 1) {
    while (right > left) {
        while (toBeSorted[left] < pivotValue) { left++; }
        while (toBeSorted[right] > pivotValue) { right--; }

        if (toBeSorted[right] < toBeSorted[left]) {
            std::swap(toBeSorted[right], toBeSorted[left]);
            left++;
            right--;
        }
    }
    } else {
        if (toBeSorted[right] < toBeSorted[left]) {
            std::swap(toBeSorted[right], toBeSorted[left]);
        }
    }
    return 0;
}

int QuickSort::getMedian(int left, int right) {
    int med = (right - left)/2;
    // if there are an even number of elements, instead of truncating
    // goto the rightmost value.
    if ((right - left)%2 != 0) {
        med = (right-left)/2 + 1;
    }

    // Organise the elements such that 
    // values at indexes: left <= med <= right.
    if (toBeSorted[med] < toBeSorted[left]) {
        std::swap(toBeSorted[left], toBeSorted[med]);
    }
    if (toBeSorted[right] < toBeSorted[left]) {
        std::swap(toBeSorted[left], toBeSorted[right]);
    }
    if (toBeSorted[right] < toBeSorted[med]) {
        std::swap(toBeSorted[right], toBeSorted[med]);
    }

    return toBeSorted[med];
}

void QuickSort::print() {
    for (int i = 0; i != toBeSorted.size(); i++) {
        std::cout << toBeSorted[i] << ",";
    }
    std::cout << std::endl;
}

int main() {
    std::vector<int> values = {5, 8, 7, 1, 2, 5, 3};
    QuickSort *sorter = new QuickSort(values);
    sorter->sortVector();
    sorter->print();
    return 0;
}
5
  • 1
    Can you include the implementation of getMidPiv function? Commented May 15, 2018 at 8:06
  • 1
    Also just an advice: Passing [begin, end) iterators of the range to partition is more C++ like than passing indices and using a vector defined under some nonlocal scope of the function. Commented May 15, 2018 at 8:09
  • You need to include a complete working example. You can't just say the output is 4 1 9 2, we need to know the inputs and the initial state of the variables here... Btw why are there two arrays listOfNums and data ? Commented May 15, 2018 at 8:28
  • @eozd I've added the getMidPiv code now, and made a few corrections, apologies made some copying errors when I tried to simplify it. Commented May 15, 2018 at 9:03
  • This is still not a Minimum, Complete and Verifiable Example. What is listOfInts, pivot? With so many nonlocal variables like this, it is hard to reason about the code and find the errors. (EDIT: Apparently there is no data now.) Commented May 15, 2018 at 9:10

2 Answers 2

1

In partition method, you shouldn't swap(data[low], data[high]) in every iteration. Your mistake is this portion. You can do it like this:

void partition(int left, int right) {
  // listOfNums is a vector
  int middle = getMidPiv(left, right);
  int low = left;
  int high = right-1;

  while (low < high) {
    while (listOfNums[low] < middle) {
      lower++;
    }

    while (listOfNums[high] > middle) {
      high--;
    }

    if (low < high) {
       swap(data[low], data[high]);
       low++;
       high--;
    }
  }

  // swap(data[low], data[high]); it is incorrect
  return low;
}
Sign up to request clarification or add additional context in comments.

1 Comment

Partition does not sort parts, it only ensures every number in left part < every number in right part. So, result is acceptable for partition, part, even though you are suffering from weird implelentation. Normally pivot is not moved out of scope and back, it just sits in its place and regular array element, and naturally takes its final place in partitioning. Even more, it's important that pivot value is part of array in classic qsort, it might be not included into subsorts [l,q] and [p, r]
0

In the first inner while loop, use low++ instead of lower++ and change the return type of function to int.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.