-1

I am trying to code the algorithm for the knapsack problem in C++. I can't get this to work. I am getting max sum equals 0 for any input. I think the problem relies in the nested for-loop. Can someone point out what went wrong and how to fix the code ?. I would like to know also what you think about my implementation. Thanks for the help.

#include <iostream>
#include <vector>
#include <set>
#include <utility>
using namespace std;
using knapsack_pair = pair<int,int>; // pair<value,weight>
using matrix_value_type = pair<int, set<knapsack_pair>>; // pair<max, set of elements>

matrix_value_type max(matrix_value_type first_value, matrix_value_type second_value){
      if(first_value.first >= second_value.first)
         return  first_value;
      else return second_value;
}

int main(){
    int number_of_values;
    int knapsack_size;
    cout << "How many values are you going to enter?" << endl;
    cin >> number_of_values;
    cout << "what's the size of the knapsack?" << endl;
    cin >> knapsack_size;
    int counter = 0;
    vector<knapsack_pair> knapsack_vector;
    while(counter < number_of_values){
        knapsack_pair new_pair;
        cout << "insert the value of the element" << endl;
        cin >> new_pair.first;
        cout << "insert the weight of the element" << endl;
        cin >> new_pair.second;
        knapsack_vector.push_back(new_pair);
        ++counter;
    }
    matrix_value_type matrix[number_of_values + 1][knapsack_size + 1];
    for(int i = 0; i < knapsack_size + 1; ++i){
        set<knapsack_pair> empty_set;
        matrix_value_type value(0,empty_set);
        matrix[0][i] = value;
    }

    for(int x = 1; x < number_of_values + 1; ++x){
        knapsack_pair current_pair = knapsack_vector.at(x-1);
        for(int y = 0; y < knapsack_size + 1; ++y){
            matrix_value_type first_value = matrix[x-1][y];
            matrix_value_type second_value;
            int weight_s_pair = y - current_pair.second;
            if(weight_s_pair >= 0){
               second_value = matrix[x-1][weight_s_pair];
               second_value.first = second_value.first + current_pair.first;
               second_value.second.insert(current_pair);
            }
            else second_value.first = -999;
            matrix[x][y] = max(first_value,second_value);
        }
    }

    matrix_value_type result = matrix[number_of_values][knapsack_size];
    cout << "the max value is: " << result.first << endl;
    cout << "the elements in the knapsack are: " << endl;
    for(auto& it : result.second)
        cout << "element of value: " << it.first << " and weight: " << it.second;
}   
patzi
  • 353
  • 4
  • 13
  • " I can't get this to work" - are you getting an error (share it with us!) Are you getting wrong results? (share them too!) – Eggcellentos Jan 07 '18 at 09:21
  • @Dan can you post an Input and the desired output as well, it will be helpful to solve this code that way – zenwraight Jan 07 '18 at 09:46
  • `1 1 1 1` does not produce 0, it produces 1 as it should (as do many other simple cases). I suspect that you're using big, un-systematic test cases. Start with small cases where you can easily figure out the desired result in advance. Increase the complexity of the test cases until you find an error. Then you can start debugging. – molbdnilo Jan 07 '18 at 10:34
  • 1
    BTW: variable-length arrays are a non-standard extension and your code will break for large arrays. Use `std::vector`. – molbdnilo Jan 07 '18 at 10:36

1 Answers1

0

I have a little problem following logic in your code. There has to be differentiated if you want to search only for the maximal value or if you want also to print the elements that build this maximal value. For just searching the optimal value I provide some updated code.

#include <iostream>
#include <utility>
#include <vector>
#include <algorithm>

using namespace std;

using WeightValue = pair<int, int>;
using WeightValueList = vector<WeightValue>;

int main() {

  int knapsack_weight;
  cout << "what's the weight of the knapsack?" << endl;
  cin >> knapsack_weight;

  int number_of_elements;
  cout << "How many elements are you going to enter?" << endl;
  cin >> number_of_elements;

  WeightValueList elements(number_of_elements);

  for (int i = 0; i < number_of_elements; ++i) {
    int w,v;
    cout << "insert the weight of the element" << endl;
    cin >> w;
    cout << "insert the value of the element" << endl;
    cin >> v;
    elements[i] = make_pair(w,v);
  }

  int mat[number_of_elements+1][knapsack_weight+1];

  for (int i = 0; i <= number_of_elements; ++i)
    for (int j = 0; j <=knapsack_weight; ++j)
      mat[i][j] = 0;

  for (int i = number_of_elements-1; i >=0; --i)
    for (int j = 1; j <= knapsack_weight; ++j) {

      int wi = elements[i].first;
      int vi = elements[i].second;

        if (wi <= j) {  // -> check if we can to better for weight j
                          // by try choosing the i-th element
          mat[i][j] = max(vi + mat[i+1][j-wi], mat[i+1][j]);
        } else {          // if the weight of i is too big, then just 
                          //copy the previously found optimum
          mat[i][j] = mat[i+1][j];
        }
    }

  cout << "The maximum value is : " << mat[0][knapsack_weight] << endl;
}
StPiere
  • 4,113
  • 15
  • 24