C++ std::vector in constructor

问题:

I am trying to code an effective implementation of the following composite class:

class composite{
  vector<base_class *> Vec;
  //Other useful constants
public:
  composite(vector<base_class*>);
  //Other useful operations...
};

My question is about the constructor and instantiation of the class and in particular the object Vec. At the minute, I use the rather crude implementation outlined below. I the implementation to be memory efficient. I'm pretty much a newb with C++, so I'm not sure I have the optimal solution here...

I use polymorphism to store different derived classes in a vector, e.g.

vector<base_class *> Vec1;
Vec1.reserve(2);
class1 * C1 = new class1(....);
Vec1.push_back(C1);
class2 * C2 = new class(....);
Vec1.push_back(C2);

where class1 and class2 are derived classes of base_class. I then pass Vec1 to the constructor of composite as follows:

composite::composite(vector<base_class*> Vec1){
   Vec.reserve(Vec1.size());
   Vec.swap(Vec1);
   //etc...
}

My feeling is that this is quite efficient on the memory, because Vec1 will be empty after the construction (it's elements have been swapped into Vec). On the other hand, it seems to be quite wasteful, as I am essentially copying the Vec1 into Vec. Is there a better way for me to do this? Can I somehow embed the vector Vec1 into composite? Thanks in advance!

回答1:

First, use proper smart pointer instead of raw pointer.

Next - in the method you used, the reserve() call is totally unnecessary - swap() just swaps internal pointers.

And last - since we're in 2013, C++11 is already to be used, so the constructor should look like this:

composite::composite(std::vector<std::unique_ptr<base_class>> v) 
    : vec{ std::move(v) }
{
}

Why this way? Taking the parameter by value already copies it, and since you aren't going to use that copy anymore, it is safe to be moved out, which achieves the least amount of copies to initialize the member.

回答2:

If you really care about whether a copy of any vector will be made or not, you should first pass the constructors argument by reference. So the "usual" implementation would look like this:

composite::composite( const vector<base_class*>& Vec1 )
  : Vec( Vec1 )
{
}

This will omit already one copy. I wouldn't bother about this until you have some signs that this will cause any problem. You already did three dynamic memory allocations before, why do you care about a fourth?