1

I need to set up and access a two dimensional vector of structure in C++. My structure is defined as:

struct nodo{
  int last_prod;
  int last_slot;
  float Z_L;
  float Z_U;
  float g;
  bool fathomed;
};

I defined the vector as:

vector<vector<struct nodo> > n_2;

Now,I need to create several elements of n_2, which will then be vectors again, and then access the single members of them. How can I achieve that? that is the piece of code I have so far:

for(int i=1;i<112;i++){
    n_2.push_back(vector<struct nodo>(111-i));       
    for(int j=1;j<112-i;j++){
      n_2[i][j].last_prod=j;
    }
}

which doesn't work.

marcob8986
  • 153
  • 4
  • 12
  • 2
    Change `vector()` to `vector(112-i)` (or just `vector(112-i)` -- this isn't C). – ildjarn Feb 20 '12 at 16:57
  • in general, I'd suggest emulating the 2D arrays with just a `vector` and compute the indices: `(i,j)---> [i*num_elements + j]` – ev-br Feb 20 '12 at 16:57
  • It's usually preferable to not use actual multidimensional arrays; they cause more trouble than they're worth. Instead, it's better to use a single array whose size is the product of the dimensions (i.e. w * h), and providing functions to access it at a given position (i.e. `GetValueAt(1, 5)`). – Collin Dauphinee Feb 20 '12 at 16:58

7 Answers7

3

A vector is size 0 until you tell it to resize, or unless you initialize it with a specific size. Pass the size of your vector in when you create it:

for(int i=1;i<112;i++){
    n_2.push_back(vector<struct nodo>(112-i));       
    for(int j=1;j<112-i;j++){
      n_2[i][j].last_prod=j;
    }
}

Also, it looks like you are skipping over the 0th index, which means your first value in your array will be skipped. That is probably undesired.

Finally, if your array is a constant size, consider using std::array rather than std::vector. Note that std::array is a C++11 feature and may not be available depending on your compiler.

If I were writing this code, I'd probably write it like this:

#include <array>
using namespace std;

// allocate an array of 112 <struct nodo> arrays, each of size 112
array<array<struct nodo, 112>, 112> n_2;
for (int i = 0; i < 112; i++)
{
    for (int j = 0; j < 112; j++)
    {
        n_2[i][j].last_prod = j;
    }
}

Or alternatively, if I don't have a compiler that supports C++11:

#include <vector>
using namespace std;

// allocate a vector of size 112 of <struct nodo> vectors, each of size 112
vector<vector<struct nodo> > n_2(112, vector<struct nodo>(112));
for (int i = 0; i < 112; i++)
{
    for (int j = 0; j < 112; j++)
    {
        n_2[i][j].last_prod = j;
    }
}

Even more ideally, you should use a 1 dimensional vector, and simply treat it as a 2 dimensional vector. This way, you can do a single allocation of memory all at once rather than 112 smaller allocations. This is getting pretty nitpicky but obviously a O(1) solution is better than an O(n) solution which is better than a O(n^2) solution in terms of allocations since allocations are slow.

Andrew Rasmussen
  • 14,912
  • 10
  • 45
  • 81
  • skipping the 0th index is done on purpose for the sake of simplicity in reading solutions. but there's still some problem left, as it doesn't work – marcob8986 Feb 20 '12 at 17:00
  • 5
    Oh how I wish I could downvote comments... "skipping the 0th index is done on purpose for the sake of simplicity in reading solutions"--bad idea. It is much better to actually learn how the language works than to attempt to force it to work "intuitively". If you try to do it this way then you have to do other odd things such as creating vectors one larger than you need. You will simply have to create a new abstraction on top of the existing framework, and this will trip you up eventually. – N_A Feb 20 '12 at 17:02
  • It may be simple in terms of you being able to read it, but simple in terms of every other C programmer in the world it will not be. My code should work, if you're still running into issues, it has nothing to do with the code in your question. – Andrew Rasmussen Feb 20 '12 at 17:22
  • 1
    Note that [Boost.Array](http://www.boost.org/libs/array/) can substitute for `std::array<>` in C++03 as well. – ildjarn Feb 21 '12 at 01:33
  • Thanks for the tidbit of information! Do you know if they actually used Boost.Array for the std::array<> implementation in C++11? – Andrew Rasmussen Feb 21 '12 at 07:23
  • 1
    Yes, but there's not much that could be done differently anyway, since the idea is to keep it an aggregate type (and that comes with quite a set of limitations). – ildjarn Feb 21 '12 at 16:24
0

In the outer loop, you're creating an empty vector (UPDATE: after the question changed, it's no longer empty, but still isn't big enough); you need to create it with a large enough size:

n_2.push_back(vector<struct nodo>(112-i));
Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
0

First of all, this isn't a very good way to do it. I'd actually suggest you use a normal 2-d array by this point if you know the dimensions in advance. Or, to stay "C++", you can wrap the vectors in something and provide a more intelligible interface.

Secondly, you should start your array indices from 0 really if you plan on directly accessing your vectors without an iterator.

But, if you really want to do this, your problem is you need to populate the vector before referencing its element.

So,

n_2.push_back(vector<struct nodo>());
n_2[0].push_back(nodo());

//You can now access n_2[0][0] as there is a single nodo element in there.

You'll need to ensure you've added an element to the nested vector before you access it the position where that element would be. So, your code could be:

for(int i=0;i<112;i++){
    n_2.push_back(vector<struct nodo>());       
    for(int j=0;j<112-i;j++){
      //At n_2[i] which we made above, add another nodo.
      n_2[i].push_back(nodo());
    }
}
John Humphreys
  • 37,047
  • 37
  • 155
  • 255
  • You should really resize, or ideally initialize the vector to a certain size. In your case, you are doing 112 * 112 (or O(n*m)) allocations, whereas passing the size into the vector on initialization is doing 112 or (O(n)) allocations all at once. – Andrew Rasmussen Feb 20 '12 at 17:31
  • 1
    I agree completely, it won't be efficient.. I didn't want to go into explaining how vectors resize by a multiple, and how memory copying will be required though. – John Humphreys Feb 20 '12 at 17:36
0

I would recommend you to use resizes, for me at least it looks neater:

n_2.resize(112);
for(int i = 1; i < 112; i++){
    n_2[i].resize(112- i);  
    for(int j = 1; j < 112 - i; j++){
      n_2[i][j].last_prod = j;
    }
}
Boris Strandjev
  • 46,145
  • 15
  • 108
  • 135
0

The first thing you must pay attention is the starting index that (in both cycles) must start from 0 instead of 1.

The other thing is that you must push something inside the nested vector in order to address it (with the operator []).

N_A
  • 19,799
  • 4
  • 52
  • 98
Riccardo Tramma
  • 583
  • 3
  • 8
  • This is wrong. You do not *have to* push something inside the nested vector. You can resize it to allocate a certain number of elements without pushing. Or, even better, you can initialize with the specific size in mind. – Andrew Rasmussen Feb 20 '12 at 17:28
0
for(int i=1;i<112;i++){
    n_2.push_back(vector<struct nodo>(111-i)); 
    // Here you only have n_2[0] ... n_2[i-1] ( the last pushed is n_2[i-1] )
    // So n_2[i] will be out of range
    // And for n_2[i-1], you only have n_2[i-1][0] ... n_2[i-1][110-i]
    // So be careful of the j's value    
    for(int j=1;j<112-i;j++){
      n_2[i][j].last_prod=j;
    }
}
Ade YU
  • 2,292
  • 3
  • 18
  • 28
0

You're trying to access items that don't exist yet.

for(int i=1;i<112;i++){
    n_2.push_back(vector<struct nodo>()); <--allocates an empty vector    
    for(int j=1;j<112-i;j++){
      n_2[i][j].last_prod=j; <-- accesses indexes in the empty vector
    }
}

Either allocate the items in the vector ahead of time or create then as needed. A better alternative is to use an array instead of a vector since you don't resize the vector anyway. This will have a performance improvement.

N_A
  • 19,799
  • 4
  • 52
  • 98