CODEWITHSUNDEEP
c++sortingstack-overflowquicksort
Moeb
Moeb

Reputation: 10861

My quicksort code overflows the stack

The code works good for small num_items, but overflows the stack for num_items = 1000.

I think 1000 is a pretty small number, so there should be a way to make this work upto much larger numbers. What can I do to make it survive more recursive calls?

#include <iostream>
#include <algorithm>
#include <vector>
#include <cstdlib>
#include <ctime>

void printv(std::vector<int>& v) {
    for (int i=0; i<v.size(); i++) std::cout << v[i] << " ";
    std::cout << std::endl;
}

void quicksort(std::vector<int>& v, int begin, int end) {

    if (((end - begin) <= 1) ||
        (begin < 0) ||
        (end > (v.size() - 1))) return;

    int pivot = v[std::rand() % v.size()];

    int x = 0;
    int y = v.size() -1;

    while(x < y) {
        int changed = 0;
        if(v[x] <= pivot) {
            x++;
            changed = 1;
        }
        if (v[y] > pivot) {
            y--;
            changed = 1;
        }
        if (!changed){
            std::swap(v[x], v[y]);
            x++;
            y--;
        }
    }

    if (x == y) y++;
    if (x > y) std::swap(x, y);

    quicksort(v, begin, x);
    quicksort(v, y, end);
}

int ran() {
    return std::rand() % 100;
}

int main() {
    std::srand(std::time(0));
    int num_items = 1000;
    std::vector<int> v (num_items);
    std::generate_n(v.begin(), num_items, ran);

    printv(v);
    quicksort(v, 0, v.size()-1);
    printv(v);
}

General comments about the code are also welcome.

Stuck at [0,4]:

#1  0x0000000000400b4c in quicksort (
    v=std::vector of length 1000, capacity 1000 = {...}, begin=0, end=4)
    at quicksort.cpp:14
#2  0x0000000000400cc1 in quicksort (
    v=std::vector of length 1000, capacity 1000 = {...}, begin=0, end=4)
    at quicksort.cpp:43
#3  0x0000000000400cc1 in quicksort (
    v=std::vector of length 1000, capacity 1000 = {...}, begin=0, end=4)
    at quicksort.cpp:43
#4  0x0000000000400cc1 in quicksort (
    v=std::vector of length 1000, capacity 1000 = {...}, begin=0, end=4)
    at quicksort.cpp:43
#5  0x0000000000400cc1 in quicksort (
    v=std::vector of length 1000, capacity 1000 = {...}, begin=0, end=4)
    at quicksort.cpp:43
#6  0x0000000000400cc1 in quicksort (
    v=std::vector of length 1000, capacity 1000 = {...}, begin=0, end=4)
    at quicksort.cpp:43
#7  0x0000000000400cc1 in quicksort (
    v=std::vector of length 1000, capacity 1000 = {...}, begin=0, end=4)
    at quicksort.cpp:43

Upvotes: 1

Views: 209

Answers (1)

fredoverflow
fredoverflow

Reputation: 263128

int pivot = v[std::rand() % v.size()];

int x = 0;
int y = v.size() -1;

You explicitly pass begin and end positions, and yet you choose the pivot element from the complete list. That doesn't sound right. You should choose a pivot element between begin and end. Also, x should start at begin, and y should start at end. Otherwise, you are always processing the complete list in each recursive step, which would explain the infinite recursion you are experiencing.

Upvotes: 4

Related Questions