Stack Overflow for Fractional Knapsack

Question

I am fairly new to C++. I have implemented the fractional knapsack problem in c++ for the course "Algorithmic Toolbox" on Coursera:

#include <iostream>
#include <iomanip>

using namespace std;

int get_max_index(double A[], double B[],int l)
{
    /*
    int A = array of value
    int B = array of weights
    int l = length of the array
    */
    int p,Max{0};
    for(int j=0;j<l;j++)
    {
        if((A[j]/B[j]) > Max){Max = A[j]/B[j];p = j;}
    }
    return p;
}

int main()
{
    int n,W,q,Max{0},W1{0};
    cin >> n >> W;
    double values[n],weights[n],loot{0};
    for(int i=0;i<n;i++)
    {
        cin >> values[i] >> weights[i];
    }
    for(int j=0;j<n;j++)
    {
        if(W==0){break;}
        else
        {
            q = get_max_index(values,weights,n);
            if(weights[q] <= W){W1 = weights[q];}
            else{W1 = W;}
            loot += W1 * (values[q]/weights[q]);
            W -= W1;
            weights[q] -= W1;
            if(weights[q] == 0){values[q] = 0;}
        }
    }
    cout << setprecision(4) << fixed;
    cout << loot << endl;
}

After submitting this code I got a stack overflow error (unknown signal 11). Please help me understand why this happens and solution to this problem.

EDIT :

I have changed the code. I am not using the get_max_index function with dynamically sized arrays. Here is the new code:

#include <iostream>
#include <iomanip>

using namespace std;


int main()
{
    long long int n,W,q,p,Max{0},W1{0};
    cin >> n >> W;
    long double values[n],weights[n],loot{0},VPU[n];
    for(long long int i=0;i<n;i++)
    {
        cin >> values[i] >> weights[i];
        VPU[i] = values[i] / weights[i];
    }
    for(long long int j=0;j<n;j++)
    {
        if(W==0){break;}
        else
        {
            for(long long int k=0;k<n;k++)
            {
                if(VPU[k] > Max){Max = VPU[k];p=k;}
            }
            Max = 0;
            q = p;
            if(weights[q] <= W){W1 = weights[q];}
            else{W1 = W;}
            loot += W1 * (values[q]/weights[q]);
            W -= W1;
            weights[q] -= W1;
            if(weights[q] == 0){VPU[q] = 0;}
        }
    }
    cout << setprecision(4) << fixed;
    cout << loot << endl;
}

`double values[n],weights[n],loot{0};` should be avoided: [https://stackoverflow.com/questions/1887097/why-arent-variable-length-arrays-part-of-the-c-standard](https://stackoverflow.com/questions/1887097/why-arent-variable-length-arrays-part-of-the-c-standard) — drescherjm, Mar 13 '20 at 13:38
I don't know if it is your main problem but here `if((A[j]/B[j]) > Max){Max = A[j]/B[j];p = j;}` you should test before that `B[j]` is not null or better that its absolute value is less that a given small threshold. If I understand well your code, `A[j]` and `B[j]` can be both null, and their division is UB. — Damien, Mar 13 '20 at 17:48

heapoverflow · Answer 1 · 2020-03-13T13:56:29.707

0

C++ standard doesn't allow variable-length arrays. So that it is not legal(even though some compilers might support it) to create a static array(allocated in stack) double values[n],weights[n]... with a size that is not known in compile time. The stack overflow error is most probably because of that(n is not known at compile time and a junk value that breaks your stack might be read). Instead try allocating them in heap with new double[n] syntax. Don't forget to free the array at the end.

edited Mar 13 '20 at 13:56

answered Mar 13 '20 at 13:30

heapoverflow

264
2
12

Although this is not valid in standard `c++` this may not be the issue. Well it will depend on how big `n` is and what the stack size is. – drescherjm Mar 13 '20 at 13:39
of course it's possible create variable length array where size is not known at compile time -- although it's not C++ standard compliant, many compilers support it. – SPD Mar 13 '20 at 13:42
@drescherjm My guess is that when `n` is declared, the compiler doesn't initiate it to 0 and a junk value is read. @SPD I don't know which compilers support it, but I will edit the answer. – heapoverflow Mar 13 '20 at 13:53

Stack Overflow for Fractional Knapsack

1 Answers1