0

I have a problem giving my std::vector to another class. I put data into the std::vector and put it into a class called "Mesh". And the "Mesh" comes into a "Model".

// Store the vertices
std::vector<float> positionVertices;

positionVertices.push_back(-0.5f);
positionVertices.push_back(-0.5f);
positionVertices.push_back( 0.5f);
positionVertices.push_back(-0.5f);
positionVertices.push_back(-0.5f);
positionVertices.push_back( 0.5f);

// Put them into a mesh and the mesh into a model
Mesh mesh = Mesh(positionVertices);
Model model = Model(mesh);

In the model class, I take the position vertices of the mesh back and convert it into a float[]. But it seems like that, that the way I allocate the std::vector is wrong, because when checking the std::vector in the model class, it has a size of 0.

// Store the vertices
float* dataPtr = &data[0];
glBufferData(GL_ARRAY_BUFFER, data.size() * sizeof(float), dataPtr, GL_STATIC_DRAW);

How can I bring the data correctly into the other classes?

I'm also unsure about the way the constructor for the mesh class works. Mesh.h:

// Mesh.h
class Mesh
{
public:
    std::vector<float> positionVertices;

    Mesh(std::vector<float>);
    ~Mesh();
};

Mesh.cpp:

// Mesh.cpp
Mesh::Mesh(std::vector<float> positionVertices) : positionVertices(Mesh::positionVertices)
{
}

Model.h:

// Model.h
class Model
{  
public:
Mesh mesh;
unsigned int vertexArray;
unsigned int vertexCount;

Model(Mesh);
~Model();

void storeData(std::vector<float> data, const unsigned int index, const unsigned int size);
};

Model.cpp:

// Model.cpp
Model::Model(Mesh mesh) : mesh(Model::mesh)
{ ... }
maniel34
  • 177
  • 9

1 Answers1

1
// Mesh.cpp
Mesh::Mesh(std::vector<float> positionVertices) :
positionVertices(Mesh::positionVertices) // Here's the problem
{
}

the positionVertices in the initializer list is Mesh::positionVertices, so you're assigning it to itself.

use

positionVertices(positionVertices)

Also, change 

Mesh::Mesh(std::vector<float> positionVertices) :

to 

Mesh::Mesh(const std::vector<float>& positionVertices) :

so you're not making needless copies of your vector.

Tzalumen
  • 652
  • 3
  • 16
  • I disagree with the last change: taking the `vector` by reference won't help, as the `vector` will still get copied into `Mesh::positionVertices`. One could move it, but otherwise I'd keep it as a copy so the interface reads better (the interface of Mesh says "I copy this" rather than "I might store a reference to this, so ensure you keep it alive for the entire lifetime of me", which makes a difference) – Tas Mar 21 '19 at 23:11
  • @Tas if it's not passed by const ref it will be copied from the original variable into the constructor call, then again into the member variable. I would direct your attention to https://stackoverflow.com/questions/2139224/how-to-pass-objects-to-functions-in-c – Tzalumen Mar 21 '19 at 23:13
  • I highly doubt any compiler worth its weight wouldn't omit one of those copies, and for me personally I'd rather my interface be clear a copy is taken, rather than users having to worry about a stored reference. Assuming OP isn't creating 10000 `Mesh` with vectors that are 10000 big, even if there are copies it's very likely meaningless. Regardless, these are just my opinions and I didn't downvote and your answer is still correct. – Tas Mar 22 '19 at 00:01
  • It is clear, because you *cannot* store a `const&` (without pointer shenanigans that you should not be doing), you can only read and copy from it. This is why I believe your opinion to be misguided, this is the gist of the article I linked to, and why I linked that article. – Tzalumen Mar 25 '19 at 17:13
  • Of course you can store a `const std::vector&`, why wouldn't you be able to? You have a reference to the original `vector`, which when it changes you receive those changes, but you are unable to change it. So it becomes problematic if a local vector is made, then given to an instance of `Mesh` which outlives the `vector`. Of course in any given small example things will be fine, but I have professionally encountered issues with passing references. For any given function they're preferred, but into a constructor is a bad idea since the class may store a reference. – Tas Mar 25 '19 at 21:58
  • I've contrived a [small example](http://coliru.stacked-crooked.com/a/ef1bf6526c2c7e3a) for you to see. Of course it's very forced, but that situation can very easily occur in a large program where you have thousands of objects which can outlive locals made – Tas Mar 25 '19 at 21:59
  • @Tas If you take a function variable that is `const std::vector& foo` and then do `std::vector bar = foo;` it will make a copy. If you do `const std::vector& bar = foo;` it will make a duplicate const&, and if you do a `std::vector& bar = foo;` the compiler will yell at you for not maintaining the const. If at any time the idea enters your head to type `&foo`, that's pointer shenanigans, stop that. Also the compiler will yell about dropping const. All of this I just tested, to make sure I was correct. – Tzalumen Mar 25 '19 at 22:10
  • I'm not disputing any of that, I agree entirely. I'm speaking only that for a CONSTRUCTOR (this doesn't apply to other functions) the class could take a reference to the object passed (as shown in my example), and that then the caller needs to ensure that object passed outlives the new object, which is undesirable. You can see in my example that `p` outlives `v`, therefore it's bad. – Tas Mar 25 '19 at 22:14
  • Actually, in your example, p.v does not contain garbage. It's empty in Debug, and full in Release, which makes it most likely undefined behavior. You *use* references in C++, you do not *store* them, that's bad practice. – Tzalumen Mar 26 '19 at 14:42