When should a function take a pointer for a collection to fill vs returning a pointer with a filled collection?

Question

In C++ I frequently see these two signatures used seemingly interchangeably:

void fill_array(Array<Type>* array_to_fill);
Array<Type>* filled_array();

I imagine there is a subtle difference, but I don't know what it is. Could someone explain when I might prefer one form over the other?

Answer 1

The first kind of signature is usually preferable.

The difference is that the second signature requires the array to be created inside the function. In particular, the second signature effectively requires the array to outlive the scope in which it was created. So what we're really comparing are these two snippets:

function foo1() {
    Array<Type>* array = /* allocate memory and call constructors */
    fill_array(array);
    do_stuff_with_array(array);
    /* free memory and call destructors */
}

function foo2() {
    Array<Type>* array = filled_array(); /* allocation/constructors happen elsewhere */
    do_stuff_with_array(array);
    /* free memory and call destructors */
}

The second version is potentially problematic for a few reasons:

1. It's error prone. Functions which return pointers or references to something they created are very easy to get wrong, either in the form of undefined behavior or in the form of a completely unnecessary performance loss. Since you're working with raw pointers, it's easy to invoke undefined behavior by returning a pointer to a local variable that's no longer valid after the function has returned. If the array was being passed around as a regular object or a reference instead, you might suffer an expensive copy when filled_array() returns (the details of when this may or may not happen are complicated, see StackOverflow for all the gory details).

2. You don't know how filled_array() allocated the memory for the array, so in principle you don't know how to deallocate it correctly. You may be able to get away with assuming it was allocated "normally", but if you don't control the allocation yourself, you just don't know for sure. It's possible some custom allocator was used, and it's also possible the pointer was saved somewhere so that a totally different function can do the deallocation at a specific time later in the pipeline (I believe this is common in C libraries). While a function that accepts a pointer as an argument could theoretically do this, it's far less likely.

3. Memory/object reuse. What if I already have memory allocated for an Array, or an actual Array, when it comes time to call filled_array()? Unfortunately, filled_array() controls both the memory allocation and the value generation logic, so it's going to allocate more memory whether or not you need it. If you have many functions like this in a row, you're potentially wasting a huge amount of time and memory on allocations that could be completely skipped if you instead accepted pointers or references to memory controlled by client code. Or more concisely: Avoid writing functions that decide how memory should be managed and do something else with that same memory. Single responsibility principle and all that.

Of course, you should be passing the Array around by reference rather than pointer. And you should be using RAII objects (whether that means "just an Array" or a smart pointer to an Array) as much as possible so that all the allocation and deallocation is managed for you. But these arguments for creating the object at the correct scope still apply, since switching to references and RAII objects alone may only change correctness bugs into performance "bugs" (some of which move semantics can't automagically fix).

Answer 2

The correct answer is:

C. None of the above.

---

Option A:

void fill_array(Array<Type>* array_to_fill);

This is more idiomatic for pre-C++11 code where smart pointers were troublesome due to a lack of move semantics, and still continues to be the safer of the two options.

The key here is the function does not "own" the memory: it performs one task and one task only. It simply fills the array. Ownership is beyond the scope of what it does and it is mostly safe.

---

Option B:

Array<Type>* filled_array();

This creates an object inside the function. There are generally two ways to do this:

1. Return a pointer to a local (stack) object. This pointer will not be valid for long and will break something (UB).
2. Return a pointer to a dynamically allocated (heap) object. Who owns the object? Where and when is its memory freed? In some architectures this can pose additional problems. For example, the Windows platform does not allow memory allocated in one module to be freed in another (i.e. cannot allocate in DLL 1 and free in DLL 2).

---

Option C:

 std::unique_ptr<Array<Type>> filled_array();

This is the absolute correct, efficient, guaranteed-to-work way.

The memory is owned by the smart pointer, and will be freed when the pointer goes out of scope (RAII) and has no other pointer to hand it off to.

Inside the function, there will be a smart pointer assigned a new array instance. The smart pointer owns that array pointer.

The function then invokes behavior through that pointer, adding data.

When the function is done, it returns the smart pointer. Since the smart pointer goes out of scope, it destructs. However, it moves the pointer it contains to a new instance in the calling location. (Note: the compiler may be able to construct the smart pointer in-place in the calling location. Regardless, smart pointers are extremely lightweight and the difference is minimal).

Now you have a smart pointer in the calling location that owns the array it points to, and that array will be freed according to the standard RAII rules. There are no dangling pointers, no memory leaks, just code that does exactly what it appears to.

Answer 3

It seems most likely that the second one returns Array<Type> and not Array<Type>*. In the first case, there is an Array<Type> somewhere and you pass a pointer to it, so the function can fill it. In the second case, the function creates an object and returns it (unless the type is Array<Type>* and I don't know what's going on).

If you use constructors with rvalue reference, the second is just as efficient but much more readable. Without rvalue references (older C++ versions) the first version was much more efficient, because the second needs to create a completely pointless copy of the value returned.

Clarification: I'm assuming that the second function returns an Array, not a pointer to an Array. Returning a pointer to an array would mean there must be an Array somewhere. So where is it? It doesn't make sense. But if the function returns an Array, that Array would most likely be created as a local variable inside the function. Then an assignment constructor is called to copy it to an object inside the calling function, and the original is destroyed. And there is the copy that you can avoid in new C++ versions.

Answer 4

You have already an accepted answer, but I am adding a new one (because I disagree with what @ixrec said).

I imagine there is a subtle difference, but I don't know what it is. Could someone explain when I might prefer one form over the other?

Ideally (in a perfect world), you should use the second form, for three reasons:

• it composes
• it naturally uses the return value to return the value that the function computes
• it is more idiomatic to write client code

Example, for these functions (note: I moved them to reference and value instead of pointers):

void fill_array(Array<Type>& array_to_fill);
Array<Type> filled_array();
void use_array(const Array<Type>& x); // this is the client code

// first call
Array<Type> a;
fill_array(a);
use_array(a);

// second call (more idiomatic)
use_array( fill_array() );

In the real world (as opposed to "Ideally"), the second form can have a few problems:

• it may be that returning the created array is prohibitive (for example, for object that do not support moving)
• it may be that (even with moving) returning by value can be expensive (consider the moving of an extremely large array)

• it disallows the transmission of error information, in code bases that do not support exceptions:

  In such a code base (not supporting exceptions), the first form of the function doesn't return void, but an error code. With the second form, the error code cannot be returned (so the second form tends to be unacceptable if you cannot throw from the function).

If your code suffers from none of these restrictions, use the first form. If it has these restrictions (no exceptions, expensive to pass the array around, or need to allways fill an existing array) use the second form.

It is also a solution to implement both functions, one in terms of the other.