NOTE: Last year, I posted three new GotWs numbered #103-105. I decided leaving a gap in the numbers wasn’t best after all, so I am renumbering them to #89-91 to continue the sequence. Here is the updated version of what was GotW #104.

 

What should factory functions return, and why?

 

Problem

While spelunking through the code of a new project you recently joined, you find the following factory function declaration:

widget* load_widget( widget::id desired );

JG Question

1. What’s wrong with this return type?

Guru Questions

2. Assuming that widget is a polymorphic type, what is the recommended return type? Explain your answer, including any tradeoffs.

3. You’d like to actually change the return type to match the recommendation in #2, but at first you worry about breaking source compatibility with existing calling code; recompiling existing callers is fine, but having to go change them all is not. Then you have an “aha!” moment, realizing that this is a fairly new project and all of your calling code is written using modern C++ idioms, and you go ahead and change the return type without fear, knowing it will require few or no code changes to callers. What makes you so confident?

4. If widget is not a polymorphic type, what is the recommended return type? Explain.

 

Solution

1. What’s wrong with this return type?

First, what can we know or reasonably expect from the two-line problem description?

We’re told load_widget is a factory function. It produces an object by “loading” it in some way and then returning it to the caller. Since the return type is a pointer, the result might be null.

The caller will reasonably expect to use the object, whether by calling member functions on it or passing it to other functions or in some other way. That isn’t safe unless the caller owns the object to ensure it is alive—either the caller gets exclusive ownership, or it gets shared ownership if the factory also maintains an internal strong or weak reference.

Because the caller has or shares ownership, he has to do something when the object is no longer needed. If the ownership is exclusive, he should destroy the object somehow. Otherwise, if the ownership is shared, he should decrement some shared reference count.

Unfortunately, returning a widget* has two major problems. First, it’s unsafe by default, because the default mode of operation (i.e., when the caller writes whitespace) is to leak a widget:

// Example 1: Leak by default. Really, this is just so 20th-century...
// 
widget* load_widget( widget::id desired );

:::

load_widget( some_id ); // oops

The Example 1 code compiles cleanly, runs, and (un)happily leaks the widget.

Guideline: Don’t use explicit new, delete, and owning * pointers, except in rare cases encapsulated inside the implementation of a low-level data structure.

Second, the signature conveys absolutely no information other than “a widget? sure, here you go! enjoy.” The documentation may state that (and how) the caller is to own the object, but the function declaration doesn’t—it’s either exclusive ownership or shared ownership, but which? Read and remember the function’s documentation, because the function declaration isn’t telling. The signature alone doesn’t even say whether the caller shares in ownership at all.

2. Assuming that widget is a polymorphic type, what is the recommended return type? Explain your answer, including any tradeoffs.

If widget is a type that’s intended to be used polymorphically and held by pointer or reference, the factory should normally return a unique_ptr to transfer ownership to the caller, or a shared_ptr if the factory shares ownership by retaining a strong reference inside its internal data structures.

Guideline: A factory that produces a reference type should return a unique_ptr by default, or a shared_ptr if ownership is to be shared with the factory.

This solves both problems: safety, and self-documentation.

First, consider how this immediately solves the safety problem in Example 1:

// Example 2: Clean up by default. Much better...
// 
unique_ptr<widget> load_widget( widget::id desired );

:::

load_widget( some_id ); // cleans up

The Example 2 code compiles cleanly, runs, and happily cleans up the widget. But it’s not just correct by default—it’s correct by construction, because there’s no way to make a mistake that results in a leak.

Aside: Someone might say, “but can’t someone still write load_widget(some_id).release()?” Of course they can, if they’re pathological; the correct answer is, “don’t do that.” Remember, our concern is to protect against Murphy, not Machiavelli—against bugs and mistakes, not deliberate crimes—and such pathological abuses fall into the latter category. That’s no different from, and doesn’t violate type safety any more than, explicitly calling Dispose early in a C# using block or explicitly calling close early in a Java try-with-resources block.

What if the cleanup should be done by something other than a plain delete call? Easy: Just use a custom deleter. The icing on the cake is that the factory itself knows which deleter is appropriate and can state it at the time it constructs the return value; the caller doesn’t need to worry about it, especially if he takes the result using an auto variable.

Second, this is self-documenting: A function that returns a unique_ptr or a value clearly documents that it’s a pure “source” function, and one that returns a shared_ptr clearly documents that it’s returning shared ownership and/or observation.

Finally, why prefer unique_ptr by default, if you don’t need to express shared ownership? Because it’s the Right Thing To Do for both performance and correctness, as noted in GotW #89, and leaves all options open to the caller:

• Returning unique_ptr expresses returning unique ownership, which is the norm for a pure “source” factory function.
• unique_ptr can’t be beat in efficiency—moving one is about as cheap as moving/copying a raw pointer.
• If the caller wants to manage the produced object’s lifetime via shared_ptr, they can easily convert to shared_ptr via an implicit move operation—no need to say std::move because the compiler already knows that the returned value is a temporary object.
• If the caller is using any other arbitrary method of maintaining the object’s lifetime, he can convert to a custom smart pointer or other lifetime management scheme simply by calling .release(). This can be useful, and isn’t possible with shared_ptr.

Here it is in action:

// Example 2, continued
//

// Accept as a unique_ptr (by default)
auto up = load_widget(1);

// Accept as a shared_ptr (if desired)
auto sp = shared_ptr<widget>{ load_widget(2) };

// Accept as your own smart pointer (if desired)
auto msp = my::smart_ptr<widget>{ load_widget(3).release() };

Of course, if the factory retains some shared ownership or observation, whether via an internal shared_ptr or weak_ptr, return shared_ptr. The caller will be forced to continue using it as a shared_ptr, but in that case that’s appropriate.

3. […] you go ahead and change the return type without fear, knowing it will require few or no code changes to callers. What makes you so confident?

Modern portable C++ code uses unique_ptr, shared_ptr, and auto. Returning unique_ptr works with all three; returning shared_ptr works with the last two.

If the caller accepts the return value in an auto variables, such as auto w = load_widget(whatever);, then the type will just naturally be correct, normal dereferencing will just work, and the only source ripple will be if the caller tries to explicitly delete (if so, the delete line can rather appropriately be deleted) or tries to store into a non-local object of a different type.

Guideline: Prefer declaring variables using auto. It’s shorter, and helps to insulate your code from needless source ripples due to minor type changes.

Otherwise, if the caller isn’t using auto, then it’s likely already using the result to initialize a unique_ptr or shared_ptr because modern C++ calling code does not traffick in raw pointers for non-parameter variables (more on this next time). In either case, returning a unique_ptr just works: A unique_ptr can be seamlessly moved into either of those types, and if the semantics are to return shared ownership and then the caller should already be using a shared_ptr and things will again work just fine (only probably better than before, because for the original return by raw pointer to work correctly the return type was probably forced to jump through the enable_shared_from_this hoop, which isn’t needed if we just return a shared_ptr explicitly).

4. If widget is not a polymorphic type, what is the recommended return type? Explain.

If widget is not a polymorphic type, which typically means it’s a copyable value type or a move-only type, the factory should return a widget by value. But what kind of value?

In C++98, programmers would often resort to returning a large object by pointer just to avoid the penalty of copying its state:

// Example 4(a): Obsolete convention: return a * just to avoid a copy 
//
/*BAD*/ vector<gadget>* load_gadgets() {
    vector<gadget>* ret = new vector<gadget>();
    // ... populate *ret ...
    return ret;
}

// Obsolete calling code (note: NOT exception-safe) 
vector<gadget>* p = load_gadgets();
if(p) use(*p);
delete p;

This has all of the usability and fragility problems discussed in #1. Today, normally we should just return by value, because we will incur only a cheap move operation, not a deep copy, to hand the result to the caller:

// Example 4(b): Default recommendation: return the value
//
vector<gadget> load_gadgets() {
    vector<gadget> ret;
    // ... populate ret ...
    return ret;
}

// Calling code (exception-safe)
auto v = load_gadgets();
use(v);

Most of the time, return movable objects by value. That’s all there is to it, if the only reason for the pointer on the return type was to avoid the copy.

There could be one additional reason the function might have returned a pointer, namely to return nullptr to indicate failure to produce an object. Normally it’s better throw an exception to report an error if we fail to load the widget. However, if not being able to load the widget is normal operation and should not be considered an error, return an optional<widget>, and probably make the factory noexcept if no other kinds of errors need to be reported than are communicated well by returning an empty optional<widget>.

// Example 4(c): Alternative if not returning an object is normal
//
optional<vector<gadget>> load_gadgets() noexcept {
    vector<gadget> ret;
    // ... populate ret ...
    if( success )            // return vector (might be empty)
        return move(ret);    // note: move() here to avoid a silent copy
    else
        return {};           // not returning anything
}

// Calling code (exception-safe)
auto v = load_gadgets();
if(v) use(*v);

Guideline: A factory that produces a non-reference type should return a value by default, and throw an exception if it fails to create the object. If not creating the object can be a normal result, return an optional<> value.

Coda

By the way, see that test for if(v) in the last line of Example 4(c)? It calls a cool function of optional<T>, namely operator bool. What makes bool so cool? In part because of how many C++ features it exercises. Here is its declaration… just think of what this lets you safely do, including at compile time! Enjoy.

constexpr explicit optional<T>::operator bool() const noexcept;

Acknowledgments

Thanks in particular to the following for their feedback to improve this article: Johannes Schaub, Leo, Vincent Jacquet.