Efficient push_back of classes and structs

Question

I've seen my colleague do the second snippet quite often. Why is this? I've tried adding print statements to track the ctors and dtors, but both seem identical.

    std::vector<ClassTest> vecClass1;
    ClassTest ct1;
    ct1.blah = blah // set some stuff
    ...
    vecClass1.push_back(ct1);

    std::vector<ClassTest> vecClass2;
    vecClass2.push_back(ClassTest());
    ClassTest& ct2 = vecClass2.back();
    ct2.blah = blah // set some stuff
    ...

PS. I'm sorry if the title is misleading.

Answer 1

If the object is more expensive to copy after "set some stuff" than before, then the copy that happens when you insert the object into the vector will be less expensive if you insert the object before you "set some stuff" than after.

Really, though, since you should expect objects in a vector to be copied occasionally, this is probably not much of an optimization.

Answer 2

They are identical (as far as I can see). Maybe he or she does that as an idiomatic custom.

Answer 3

You should probably ask your collegue to know exactly why, but we can still take a guess. As James pointed out, it might be a tad more efficient if the object is more expensive to copy once constructed.

I see advantages in both versions.

I like your collegue's snippet because: although there are 2 objects in both cases, they only co-exist for a very short period of time in the second version. There is only one object available for editing: this avoids the potential error of editing ct1 after push_back.

I like your personal snippet because: invoking push_back to add a second object potentially invalidates the reference ct2, inducing a risk of undefined behavior. The first snippet does not present this risk.

Answer 4

The second version benefits from moving the temporary. The first version is copying the temporary vector. So the second one is potentially faster. The second version has also potentially smaller peak memory requirements, the first version creates two objects one temporary and one copy of it and only then deletes the temporary. You can improve the first version by explicitly moving the temporary:

std::vector<ClassTest> vecClass1;
ClassTest ct1;
ct1.blah = blah // set some stuff
...
vecClass1.push_back(std::move(ct1));

Answer 5

If we accept that your colleague's snippet is wise, because ClassTest is expensive to copy, I would prefer:

using std::swap;

std::vector<ClassTest> vecClass1;
ClassTest ct1;
ct1.blah = blah // set some stuff
...
vecClass1.push_back(ClassTest());
swap(ct1, vecClass1.back());

I think it's clearer, and it may well be more exception-safe. The ... code presumably allocates resources and hence could throw an exception (or else what's making the fully-built ClassTest so expensive to copy?). So unless the vector really is local to the function, I don't think it's a good idea for it to be half-built while running that code.

Of course this is even more expensive if ClassTest only has the default swap implementation, but if ClassTest doesn't have an efficient swap, then it has no business being expensive to copy. So this trick perhaps should only be used with classes known to be friendly, rather than unknown template parameter types.

As Gene says, std::move is better anyway, if you have that C++0x feature.

If we're worried about ClassTest being expensive to copy, though, then relocating the vector is a terrifying prospect. So we should also either:

• reserve enough space before adding anything,
• use a deque instead of a vector.