Confusion on push_back interaction with pair<float,int>

I have no error message instead I only have unexpected behavior.

double get_optimal_value(int capacity, vector<int> weights, vector<int> values) {
  int n = weights.size();
  vector<pair<double, int>> valuePerWeight(n);
  pair<double,int> x;
  for(int i = 0; i < n; i++){ 
    double v = values[i]/weights[i]; 
    x = make_pair(values[i]/weights[i], weights[i]);
    valuePerWeight.push_back(x);
  }

  for(int i = 0; i < n && capacity > 0; i++){
    int amount = min(capacity, valuePerWeight[i].second);
    value += valuePerWeight[i].first * amount;
    capacity -= amount;
  }

  double value = 0.0;
  return value;
}

I am creating a vector with values of type pair<double,int>. I create the pair using make_pair(some_double, some_int), then I call push_back with the pair.

Later in the function I index into the vector and do stuff using the pairs. However an issue arises, when I index into my valuePerWeight vector and retrieve the attributes of the different pairs. They all end up being zero regardless of index and regardless of .first or .second.

Through printing a bunch of variables I have asserted the created pair is not {0,0} but as soon as I push_back into the vector and index the pair and look at it's .first and .second attributes both are equal to 0.

I can't seem to understand why this is, originally I was using push_back seen as below

valuePerWeight.push_back(make_pair(values[i]/weights[i], weights[i]));

instead of creating into a temporary variable x . However the same issue still stands.

Any help in the right direction would be greatly appreciated. If there is any further clarification that I can give please ask me.

If you'd like to see for some values below is a snippet which can be compiled

I use input

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

using namespace std;

double get_optimal_value(int capacity, vector<int> weights, vector<int> values) {
  double value = 0.0;
  int n = weights.size();
  vector<pair<double, int>> valuePerWeight(n);
  pair<double,int> x;
  for(int i = 0; i < n; i++){

    double v = values[i]/weights[i];
    cout << v << ' '<< weights[i] << '\n';
    x = make_pair(values[i]/weights[i], weights[i]);
    cout << x.first << ' ' << x.second << '\n';

    valuePerWeight.push_back(x);
    cout << valuePerWeight[i].first << ' ' << valuePerWeight[i].second << '\n';
  }


  for(int i = 0; i < n; i++){
    cout << valuePerWeight[i].first;
    cout << valuePerWeight[i].second;

    cout << '\n';
  }

  sort(valuePerWeight.begin(), valuePerWeight.end());

  for(int i = 0; i < n && capacity > 0; i++){
    int amount = min(capacity, valuePerWeight[i].second);
    value += valuePerWeight[i].first * amount;
    capacity -= amount;
  }



  // for(auto vp: valuePerWeight){
  //   cout << vp.first << vp.second;
  //   cout << '\n';
  // }

  return value;
}

int main() {
  int n;
  int capacity;
  std::cin >> n >> capacity;
  vector<int> values(n);
  vector<int> weights(n);
  for (int i = 0; i < n; i++) {
    std::cin >> values[i] >> weights[i];
  }

  double optimal_value = get_optimal_value(capacity, weights, values);

  std::cout.precision(10);
  std::cout << optimal_value << std::endl;
  return 0;
}

Solution

The confusion here is due to the behavior of the constructor you use

vector<pair<double, int>> valuePerWeight(n);

This actually fills the vector with n default constructed pairs, which as you may surmise, are (0, 0). When you push_back, you push to the end of these, so you a totally get 2n pairs.

.reserve does something close to what you expected, not actually filling the vector, but is likely not needed for something not bottle-necking on vector resizing.

Short story, omit the (n) to just construct an empty vector.

Three more suggestions: accept the vectors as const& to save a copy, and look at emplace_back instead of making a pair yourself and pushing it. That's what it's meant for. Also, note the comment by churill - dividing two integers will result in integer division regardless of where you are assigning the result. Static cast one of them to a float or double (or multiply by 1.0 at the start) to ensure floating point division.