Why is creating STL containers dynamically considered bad practice?

Question

Title says it.

Sample of bad practive:

std::vector<Point>* FindPoints()
{
   std::vector<Point>* result = new std::vector<Point>();
   //...
   return result;
}

What's wrong with it if I delete that vector later?

I mostly program in C#, so this problem is not very clear for me in C++ context.

Answer 1

As a rule of thumb, you don't do this because the less you allocate on the heap, the less you risk leaking memory. :)

std::vector is useful also because it automatically manages the memory used for the vector in RAII fashion; by allocating it on the heap now you require an explicit deallocation (with delete result) to avoid leaking its memory. The thing is made complicated because of exceptions, that can alter your return path and skip any delete you put on the way. (In C# you don't have such problems because inaccessible memory is just recalled periodically by the garbage collector)

If you want to return an STL container you have several choices:

• just return it by value; in theory you should incur in a copy-penality because of the temporaries that are created in the process of returning result, but newer compilers should be able to elide the copy using NRVO¹. There may also be std::vector implementations that implement copy-on-write optimization like many std::string implementations do, but I've never heard about that.

  On C++0x compilers, instead, the move semantics should trigger, avoiding any copy.

• Store the pointer of result in an ownership-transferring smart pointer like std::auto_ptr (or std::unique_ptr in C++0x), and also change the return type of your function to std::auto_ptr<std::vector<Point > >; in that way, your pointer is always encapsulated in a stack-object, that is automatically destroyed when the function exits (in any way), and destroys the vector if its still owned by it. Also, it's completely clear who owns the returned object.

• Make the result vector a parameter passed by reference by the caller, and fill that one instead of returning a new vector.

• Hardcore STL option: you would instead provide your data as iterators; the client code would then use std::copy+std::back_inserter or whatever to store such data in whichever container it wants. Not seen much (it can be tricky to code right) but it's worth mentioning.

---

1. As @Steve Jessop pointed out in the comments, NRVO works completely only if the return value is used directly to initialize a variable in the calling method; otherwise, it would still be able to elide the construction of the temporary return value, but the assignment operator for the variable to which the return value is assigned could still be called (see @Steve Jessop's comments for details).

Answer 2

Creating anything dynamically is bad practice unless it's really necessary. There's rarely a good reason to create a container dynamically, so it's usually not a good idea.

Edit: Usually, instead of worrying about things like how fast or slow returning a container is, most of the code should deal only with an iterator (or two) into the container.