0

I have a loop which adds pointers to a vector;

vector<Material *> materials;

and my Material class has 4 attributes :

int id;
float *ambiance;
float *diffuse;

in my loop :

while(input_read_from_the_file !=NULL){
int id=someval1;
float x[2]={someval2,someval3};
float y[2]={someval4,someval5};
materials.push_back(new Material(id,x,y));
}

When I read my materials vector in a for loop I see that ids are different but ambiance and diffuse are same for all the elements.Thats probably because it uses the same pointer in the while loop but I couldnt find an alternative.What should be the best approach here? Thanks

4
  • 5
    Your pointers point to variables that are local to the while loop. This is undefined behaviour. Why not store arrays or vectors instead of pointers? Commented Oct 27, 2013 at 9:29
  • Should I use arrays instead of pointers in my class? instead of float* ambiance; float ambiance[3]; Commented Oct 27, 2013 at 9:33
  • 1
    @Emrah is there is a reasonable and known fixed number of them you'd be crazy not to store them as a declared array. There is no need to store them dynamically if that is the case. If there is such a need, use a vector instead. Commented Oct 27, 2013 at 9:36
  • Thank you very much it solved my problem.I am not crazy but a beginner :) Commented Oct 27, 2013 at 9:41

1 Answer 1

1

I'd try to avoid pointers as much as I can. Let's start with your vector. Why does it need to be vector<Material*> and not vector<Material>? Unless Material is an inherited class, you can use a vector of Material objects instead of pointers. That way, you don't need the destructor of the class that has vector<Material*> to iterate and destroy each of them (by using shared_ptr, you can avoid this too).

Now, as mentioned in the comments, the problem is that Material uses pointers for the ambiance and diffuse members. No reason for that too. Technically, you want it to be Vector3 or Vector4 when you're writing a renderer or material system, but lets go with float[2] instead.

C++11 (0x) has cool move semantics that you can use to avoid the creation of a temporary object (since we're going to push an object into the vector and without move semantics, a temporary object is created while doing so)

So, your code looks like:

class Material {
int id;
float ambiance[2]; // you really ought to use Vector2 instead. pointers are evil.
float diffuse[2];

Material (const int _id, const float _amb[], const float _dif[]) : id(_id) {
   ambiance[0] = _amb[0]; ambiance[1] = _amb[1]; // actual copy is made
   diffuse[0]  = _dif[0]; diffuse[1]  = _dif[1]; 
}
}

-----
vector<Material> materials;

while(input_read_from_the_file !=NULL){
   int id    = someval1;
   float x[2]= {someval2,someval3};
   float y[2]= {someval4,someval5};
   materials.emplace_back(Material(id,x,y)); // or even materials.emplace_back(id, x, y);
}
Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.