Unable to access elements of vector<vector<?>>

[klebermo]

I have this class where the only atribute is a vector<vector<?>> like this:

class Test {
private:
  vector<vector<struct Pixel>> pixels
public:
  ...
}

to fill pixels with data, I have this method read where first I read data from a file and put in a 1d array:

        std::stringstream ss(line_pixels);
        std::string value;
        while(getline(ss, value, ' ')) {
          pixel p;
          p.r = p.g = p.b = stoi(value);
          v.push_back(p);
        }

after that I transfer the data from this 1d vector to the 2d vector pixels. I have try to do this in this 2 ways:

    int index = 0;
    for(int i=0; i<stoi(height); i++) {
      std::vector<pixel> row;
      for(int j=0; j<stoi(width); j++) row.push_back(v[index++]);
      pixels.push_back(row);
    }

with this, when I try access any element from pixels with something like pixel[index], I got a segmentation fault error.

    for(long unsigned int i=0; i<v.size(); i++) {
      int row = i / stoi(width);
      int col = i % stoi(width);
      pixels[row][col] = v[i];
    }

with this option, before I did that, I resize the vector this way:

  pixels.resize(stoi(height));
  for(int i=0; i<stoi(height); i++) pixels[i].resize(stoi(width));

but I got an segmentation fault error in that resize code.

Anyone can tell me the correct way to do that?

[jonnin]

since this is pretty small, try change all the [] notation to .at until you find it, fix it and then restore the (faster) [] notation.

segmentation faults with vectors are just like pointers/arrays: 99% of the time you simply are off by 1 in the index or the index is uninitialized or something like that.

[Peter87]

Make sure that stoi(height) * stoi(width) == v.size().

And you probably don't want to call stoi every loop iteration. Just do it once before the loop and store the result in integer variables that you use in the rest of the code. It won't fix your problem but is more efficient because it doesn't repeat the conversion from string to int unnecessarily.

[Peter87]

jonnin wrote:

since this is pretty small, try change all the [] notation to .at until you find it, fix it and then restore the (faster) [] notation.

Or just enable the "debug mode".

With libstdc++ (GCC's implementation of the standard library) you can do that by using the -D_GLIBCXX_DEBUG compiler flag.

With libc++ (Clang's own implementation of the standard library) you can use: -D_LIBCPP_DEBUG=1

Visual C++ (Microsoft's implementation) also has a "debug mode" that I have no experience with but I've heard a lot about it.

[keskiverto]

vector<vector<struct Pixel>> does not require the struct; one can write: vector<vector<Pixel>>

Why the stoi() in multiple places?

You seem to have whitespace-delimited integers in input and should be able to use:

int value {};
while ( ss >> value ) {
  v.emplace_back( value, value, value );
}

const int cols = stoi(width);
if ( 0 < cols ) {
  const int rows = v.size() / cols;
  if ( rows * cols == v.size() ) {
    pixels.resize(rows);
    for ( int row=0; row < rows; ++row ) {
      pixels[row].assign( v.begin()+row*cols, v.begin()+(row+1)*cols );
    }
  } else {
    // v is not rows * cols
  }
} else {
  // width is not valid
}

[seeplus]

[I've already provided corrected updated code for this in previous posts bur the OP seems to insist on using their code....]

https://cplusplus.com/forum/beginner/285127/
https://cplusplus.com/forum/beginner/285133/

[klebermo]

vector<vector<struct Pixel>> does not require the struct; one can write: vector<vector<Pixel>>

Why the stoi() in multiple places?

You seem to have whitespace-delimited integers in input and should be able to use:

int value {};
while ( ss >> value ) {
v.emplace_back( value, value, value );
}

const int cols = stoi(width);
if ( 0 < cols ) {
const int rows = v.size() / cols;
if ( rows * cols == v.size() ) {
pixels.resize(rows);
for ( int row=0; row < rows; ++row ) {
pixels[row].assign( v.begin()+row*cols, v.begin()+(row+1)*cols );
}
} else {
// v is not rows * cols
}
} else {
// width is not valid
}

I have tried this, but I got an error (segmentation fault) in the line 10: pixels.resize(rows);

[Peter87]

It's hard to say why pixels.resize(rows) would fail in the above code. My guess is that either your code is different and the value of rows is negative, very huge or possibly uninitialized (print the values of width and height to see that they are what you expect), or something else is wrong (UB doesn't necessarily lead to an immediate crash - it might result in bugs or crashes in totally different parts of the program - Valgrind or UB sanitizers can help finding these issues).

[MikeyBoy]

Have you tried stepping through your code with your debugger, to examine the memory to determine where things might be going wrong?

[LtL]

Every time you found some unexpected errors or sth., give your ENTIRE piece of code and mark at least 1 error you found at, so that we can help you within minimal time.
Also, it's common for coders to find their code having errors like yours, so try to use C++ arrays instead of vectors. They're faster and simpler, and as long as their size is larger or equal to how much you've stored, no need to worry about overflow.

[Peter87]

LtL wrote:

Every time you found some unexpected errors or sth., give your ENTIRE piece of code and mark at least 1 error you found at, so that we can help you within minimal time.

I kind of agree, if the program is small, otherwise it's good if you remove the parts of your code that is not relevant for demonstrating your problem so that we don't have more than necessary to look at. This makes it much easier for us to help you. It is important that the code still compiles and produces the problem that you want to show when you run it otherwise it's pointless (it's very common that people think that a problem is in one part of the code when in reality it's somewhere else, that's often why they don't find it). If you do it well (i.e. remove all irrelevant parts) it is likely that you will find the problem yourself in the process but otherwise just post it and we'll probably be able to help.

https://en.wikipedia.org/wiki/Minimal_reproducible_example

LtL wrote:

it's common for coders to find their code having errors like yours, so try to use C++ arrays instead of vectors.

Why? Are you suggesting a raw array is less error-prone than std::vector?

Personally I find std::vector much easier to use, at least when the size is not a compile time constant (in that case they are about equal, except for things like copying).

EDIT: I admit that setting the initial size of a "2D array" (array of arrays) is easier compared to a "2D vector" (vector of vectors) when the dimensions are compile time constants. This does not seem to be the case in klebermo's code.

LtL wrote:

They're faster and simpler, and as long as their size is larger or equal to how much you've stored, no need to worry about overflow.

Are they really faster? I think it depends on how you use them.

If you are referring to an array that is not dynamically allocated (where the size is known at compile time) then the array can be slightly more efficient because you avoid the dynamic allocation (which takes some extra time) and you keep things closer together in memory (which can be good for cache locality).

But std::vector store its elements in an array internally, and you can set the size up front if you want to avoid the multiple reallocations that can otherwise happen when using push_back, so the performance can often be very similar compared to an array.

There might be reasons for why you would not want to use std::vector, but in most cases it's good enough, and especially for beginners it's unlikely that any difference in performance would matter (there will probably be other things that matters a lot more).