What is wrong with using arrays dynamically allocated in C++? [duplicate]

Question

Like the following code :

int size = myGetSize(); std::string* foo; foo = new std::string[size]; //... // using the table //... delete[] foo; 

I heard that such use (not this code precisely, but dynamic allocation as a whole) can be unsafe in some cases, and should be used only with RAII. Why?

Answer 1

I see three main problems with your code:

1. Use of naked, owning pointers.

2. Use of naked new.

3. Use of dynamic arrays.

Each is undesirable for its own reasons. I will try to explain each one in turn.

(1) violates what I like to call subexpression-wise correctness, and (2) violates statement-wise correctness. The idea here is that no statement, and not even any subexpression, should by itself be an error. I take the term "error" loosely to mean "could be a bug".

The idea of writing good code is that if it goes wrong, it wasn't your fault. Your basic mindset should be that of a paranoid coward. Not writing code at all is one way to achieve this, but since that rarely meets requirements, the next best thing is to make sure that whatever you do, it Isn't Your Fault. The only way you can systematically prove that it's not your fault is if no single part of your code is the root cause of an error. Now let's look at the code again:

• new std::string[25] is an error, because it creates a dynamically allocated object which is leaked. This code can only conditionally become a non-error if someone else, somewhere else, and in every case, remembers to clean up.

  This requires, first of all, that the value of this expression be stored somewhere. This is happening in your case, but in more complex expressions it may be hard to prove that it will ever happen in all cases (unspecified evaluation order, I'm looking at you).

• foo = new std::string[125]; is an error because again foo leaks a resource, unless the stars align and someone remembers, in every case and at the right time, to clean up.

The correct way of writing this code so far would be:

std::unique_ptr<std::string[]> foo(std::make_unique<std::string[]>(25)); 

Note that every single subexpression in this statement is not the root cause of a program bug. It Is Not Your Fault.

Finally, as for (3), dynamic arrays are a misfeature in C++ and should basically never be used. There are several standard defects relating just to dynamic arrays (and not considered worth fixing). The simple argument is that you cannot use arrays without knowing their size. You might say that you could use a sentinel or tombstone value to mark the end of an array dynamically, but that makes the correctness of your program value-dependent, not type-dependent, and thus not statically checkable (the very definition of "unsafe"). You cannot assert statically that It Wasn't Your Fault.

So you end up having to maintain a separate storage for the array size anyway. And guess what, your implementation has to duplicate that knowledge anyway so it can call destructors when you say delete[], so that's wasted duplication. The correct way, instead, is not to use dynamic arrays, but instead separate memory allocation (and make it customizable via allocators why we're at it) from element-wise object construction. Wrapping all this (allocator, storage, element count) into a single, convenient class is the C++ way.

Thus the final version of your code is this:

std::vector<std::string> foo(25);