0

I'm trying to implement Quickselect, which should find the kth smallest element in an array of ints. However, the function findKthSmallest() is returning the kth value, rather than the kth smallest. I can't figure out why this is happening - I tried a lot of small changes, and refactored and rewrote it a couple times now. It seems like it should be really simple, but I don't know. Here is my C++ code:

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

//note: when i put an 'I' at the end of a variable name, it stands for Index. (and V stands for Value)

string vecToStr(vector<int>);

class KSE {//KSE stands for Kth Smallest Element
public:
    KSE(vector<int> S, int K);
    vector<int> set;
    int k;
    int kthSmallest;
private:
    int findKthSmallest();
//  int findKthSmallest(int leftI, int rightI);
    int partition(int leftI, int rightI, int pivotI);
    void swap(int aI, int bI);
};

//KSE definitions:

KSE::KSE(vector<int> S, int K) {
    set = S;
    k = K;
    kthSmallest = findKthSmallest();
}

int KSE::findKthSmallest() {
    int leftI = 0;
    int rightI = set.size() - 1;
    int pivotI = -1;
    while (true) {
        if (leftI == rightI)
            break;//-----------------------------break

        //pivotI = rand() % (rightI - leftI + 1) + leftI - 1;//random number in the range of leftI to rightI, inclusive
        pivotI = leftI;
        cout << "pivot is " << set[pivotI] << endl;
        pivotI = partition(leftI, rightI, pivotI);

        cout << "set after partition: " << vecToStr(set) << endl;

        if (k == pivotI)
            break;
        else if (k < pivotI)
            rightI = pivotI - 1;
        else//if k > pivotI
            leftI = pivotI + 1;

        cout << "loopdone" << endl;
    }
    return set[k];
}

int KSE::partition(int leftI, int rightI, int pivotI) {
    //https://youtu.be/MZaf_9IZCrc
    int i = leftI - 1;
    for (int j = leftI; j <= rightI - 1; j++) {
        if (set[j] < set[pivotI]) {
            i++;
            swap(i, j);
        }
    }
    swap(i + 1, pivotI);
    pivotI = i + 1; 
    return pivotI;
}

void KSE::swap(int aI, int bI) {
    cout << "swapping " << set[aI] << " with " << set[bI] << endl;
    int tempV = set[aI];
    set[aI] = set[bI];
    set[bI] = tempV;
}

int main() {

    vector<int> kse_case1_set = { 2,3,5,8,12,1,7,10,13,30,6,14,15,18,22 };
    int kse_case1_k1 = 4;
    cout << "Testing Find Kth Element Algorithm" << endl << endl;
    cout << "set = " << vecToStr(kse_case1_set) << endl;

    KSE test1 = KSE(kse_case1_set, kse_case1_k1);
    cout << "when k = " << test1.k << endl;
    cout << "\tkth smallest element = " << test1.kthSmallest  << endl << endl;

    cout << "Press Enter to exit.";
    cin.get();
}

//to help with printing test cases:
string vecToStr(vector<int> vec) {
    string str = "[";
    for (int i{ 0 }; i < vec.size() - 1; i++) {
        str += to_string(vec[i]);
        str += ", ";
    }
    str += to_string(vec[vec.size() - 1]);
    str += "]";
    return str;
}

And here is what it outputs:

set = [2, 3, 5, 8, 12, 1, 7, 10, 13, 30, 6, 14, 15, 18, 22]
pivot is 2
swapping 2 with 1
swapping 3 with 1
set after partition: [3, 1, 5, 8, 12, 2, 7, 10, 13, 30, 6, 14, 15, 18, 22]
loopdone
pivot is 5
swapping 5 with 2
swapping 8 with 2
set after partition: [3, 1, 8, 2, 12, 5, 7, 10, 13, 30, 6, 14, 15, 18, 22]
loopdone
pivot is 12
swapping 12 with 5
swapping 12 with 5
set after partition: [3, 1, 8, 2, 12, 5, 7, 10, 13, 30, 6, 14, 15, 18, 22]
loopdone
when k = 4
        kth smallest element = 12

Press Enter to exit.

The 4th smallest element (starting at 0th) should be 6, not 12. Do you guys know what's going wrong?

  • 1
    Your `partition` function is wrong. You're comparing against `set[pivotI]` rather than saving the original value there and comparing against it. I don't know if that's all your problems, but that's the first one that jumped out at me. – Jim Mischel Apr 28 '20 at 05:46
  • Without looking at your code, it seems to me that if you have a working `int KthValue(int* a, int size)` function then the `int KsmallestValue(int* a, int size)` function can be coded as `{ return((size - 1) - KthValue(a, size)); }` – 500 - Internal Server Error Apr 28 '20 at 14:45
  • Thanks for the comment - I have it comparing to the saved value of set[pivotI] now, but I still have the same problem in the end. – user13419829 Apr 29 '20 at 17:06

0 Answers0