Bad practice to return unique_ptr for raw pointer like ownership semantics?

Question

I've written a static factory method that returns a new Foobar object populated from another data object. I've recently been obsessed with ownership semantics and am wondering if I'm conveying the right message by having this factory method return a unique_ptr.

class Foobar {
public:
    static unique_ptr<Foobar> factory(DataObject data);
}

My intent is to tell client code that they own the pointer. Without a smart pointer, I would simply return Foobar*. I would like, however, to enforce that this memory be deleted to avoid potential bugs, so unique_ptr seemed like an appropriate solution. If the client wants to extend the lifetime of the pointer, they just call .release() once they get the unique_ptr.

Foobar* myFoo = Foobar::factory(data).release();

My question comes in two parts:

1. Does this approach convey the correct ownership semantics?
2. Is this a "bad practice" to return unique_ptr instead of a raw pointer?

Phraseology

You're returning a unqiue_ptr to tell the client they own the pointer? This is exactly the opposite of what I would expect (since they have to explicitly take ownership of the unique pointer). – Buskus

You might want to use move-semantics instead (if you are able to use C++11). With this, it is up to the user to decide how to prolong the lifetime of the object created by the factory. – Altonaltona

@Altonaltona that's something that's done automatically for you, no? – Bartender

@SethCarnegie yes. I am wondering why bkuhns doesn't return an object of type Foobar to transfers ownership. – Altonaltona

I just wanted to reiterate what's been said is some of the answers: instead of using release(), the client code should keep the pointer as a smart pointer, converting it to a shared_ptr if shared ownership is necessary or leaving it as unique_ptr otherwise. – Alluvion

@bames53, Sorry, mentioning release() was out of context. I work on a fairly old project and I was trying to anticipate both the need and coworker concern about being able to handle a raw pointer out of the returned unique_ptr. – Phraseology

@evnu, I hadn't considered returning the type directly (by "moving" it). That seems like a very efficient concept thanks to rvalues, and addresses all my concerns without needing a smart pointer. In general, though, I would prefer a consistent factory pattern to use throughout the whole application that works with polymorphism, and I don't imagine this technique would handle that. unique_ptr, as the answers below confirm, seems to be an acceptable approach, and would allow for polymoprhism. – Phraseology

How would you handle a factory that could in normal operation fail with a move-return? (say, a parser that could fail to match a particular pattern, indicating that you should try another pattern). For such cases, it seems that unique_ptr is ideal, because the caller can test whether it was a successful match, and even if it is, can decide whether to explicitly move the pointer to a surviving object or let it die implicitly as it leaves its own scope. – Stallings

You can have a look here for typical usage of unique_ptr (especially with regards to ownership semantics). The article describes the factory pattern from your question as well as passing a unique_ptr to a function. – Stock

Answer 1

Returning a std::unique_ptr from a factory method is just fine and should be a recommended practice. The message it conveys is (IMO): You are now the sole owner of this object. Furthermore, for your convenience, the object knows how to destroy itself.

I think this is much better then returning a raw pointer (where the client has to remember how and if to dispose of this pointer).

However I do not understand your comment about releasing the pointer to extend it's lifetime. In general I rarely see any reason to call release on a smartpointer, since I think pointers should always be managed by some sort of RAII structure (just about the only situation where I call release is to put the pointer in a different managing datastructure, e.g. a unique_ptr with a different deleter, after I did something to warrant additional cleanup) .

Therefore the client can (and should) simply store the unique_ptr somewhere (such as another unique_ptr, which has been move constructed from the returned one) as long as they need the object (or a shared_ptr, if they need multiple copies of the pointer). So the clientside code should look more like this:

std::unique_ptr<FooBar> myFoo = Foobar::factory(data);
//or:
std::shared_ptr<FooBar> myFoo = Foobar::factory(data);

Personally I would also add a typedef for the returned pointer type (in this case std::unique_ptr<Foobar>) and or the used deleter (in this case std::default_deleter) to your factory object. That makes it easier if you later decide to change the allocation of your pointer(and therefore need a different method for destruction of the pointer, which will be visible as a second template parameter of std::unique_ptr). So I would do something like this:

class Foobar {
public:  
    typedef std::default_deleter<Foobar>     deleter;
    typedef std::unique_ptr<Foobar, deleter> unique_ptr;

    static unique_ptr factory(DataObject data);
}

Foobar::unique_ptr myFoo = Foobar::factory(data);
//or:
std::shared_ptr<Foobar> myFoo = Foobar::factory(data);

Isodimorphism

I didn't mean for my mention of release() to be misleading or confusing, sorry. This new code is going into a fairly large existing application (1-2 million lines of C++) that doesn't use any smart pointers (other than COM). I know others on my team will be concerned if there's not a reasonable way to get to the raw pointer if legacy code makes it effectively "required". I'll, of course, give the proper warnings and recommend sticking to unique_ptr/shared_ptr when possible. I really like your typedef idea, which sets your answer apart from James McNellis'. Thanks for the insights. – Phraseology

Answer 2

A std::unique_ptr uniquely owns the object to which it points. It says "I own this object, and no one else does."

That is exactly what you are trying to express: you are saying "caller of this function: you are now the sole owner of this object; do with it as you please, its lifetime is your responsibility."

Inconvenience

Indeed, but why not have it return a std::shared_pointer<>? It achieves the same effect but allows multiple copies of the pointer. – Preceptory

@AndréCaron: Why impose the use of std::shared_ptr if all you want to do is transfer ownership to the caller? Let the caller give ownership of the object to std::shared_ptr if it wants to (or another kind of smart pointer, if the caller wants to). – Inconvenience

@André Caron: The reason is basically performance. The factory can't know whether or not the client will the functionality added by std::shared_ptr over std::unique_ptr, so why sacrifice performance for some functionality, which might not be necessary. Since shared_ptr can be constructed and assigned to from unique_ptr there is really no benefit – Isodimorphism

The caller doesn't strictly own the pointer. The caller's unique_ptr object owns the pointer. :p – Balakirev

@AndréCaron a shared_ptr can be constructed from a unique_ptr, but the reverse is generally not possible. – Alluvion

@bames53: ah! That would indeed be a good reason. I think the idea is most cleanly expressed by Dietmar Kuhl's answer. Thanks! – Preceptory

Answer 3

It exactly conveys the correct semantics and is the way I think all factories in C++ should work: std::unique_ptr<T> doesn't impose any kind of ownership semantics and it is extremely cheap.

Redfield