Which of these is better practice?

Question

Let's say I have a class FOO.

I want to have a std::vector of FOO.

Is it better if I do something like this:

FOO foo;
foo.init();
foo.prop = 1;
std::vector<FOO> myvec;

myvec.push_back(foo);
foo.prop = 2;
myvect.push_back(foo);

or is it better practice to do:

std::vector<FOO> myvec;
FOO foo;
myvec.push_back(foo);
myvec.back().init();
myvec.back().prop = 1;

myvec.push_back(foo);
myvec.back().init();
myvec.back().prop = 2;

I'm basically not sure if its better to make a model and push in the model instead of making an instance, pushing it, then modifying it from the vector. Also, which one is safer and least likely to result in memory leaks?

Thanks

Answer 1

Best practice is not to have an init() function - you want a constructor. If you always need to set prop, give the constructor a parameter to do it. This has nothing specifically to do with vectors - it's the way that all C++ code should be written.

Answer 2

Neither method has any memory issues as you're dealing with values and aren't dynamically allocating any objects manually.

I would favour giving FOO a constructor which does whatever init does and sets prop to the appropriate value. Then you can just push the values you want:

myvec.push_back(FOO(1));
myvec.push_back(FOO(2));

Answer 3

I think the best is to:

myvec.push_back(FOO(1));
myvec.push_back(FOO(2));

Answer 4

The answer depends on what your FOO class does. If it's a simple structure with no pointers etc., then both your approaches are fine and do the same.

Note that push_back inserts a copy of the object into the vector. If your class allocates memory on the heap, you need a copy constructor that creates a deep copy of your objects, otherwise you'll end up with memory leaks. Also, if your objects are quite large, it may be inefficient to create copies. In such cases, what I generally do is allocate the object itself on the heap externally and insert the pointer into the vector:

std::vector<FOO *> myvec;
FOO *foo;

foo = new FOO();
foo->init();
foo->val = 1;
myvec.push_back(foo);

foo = new FOO();
foo->init();
foo->val = 2;
myvec.push_back(foo);

However, in this case, you need to remember to free the objects before destroying the vector.

Answer 5

Besides what others have said about initializing your objects in the constructor, I would add this:

In your second example, where you put objects in the vector and then initialize them, you risk leaving your vector in an unusable state.

If for example, the init() method can throw an exception, you'll have a non/partially initialized object in your vector.

Of course, these problems goes away with a constructor that makes sure the objects are properly initialized.

In general: Don't start doing stuff with objects before they're in a usable state.