Returning a unique_ptr from a class method C++0x

Question

If my class SomeType has a method that returns a element from the map (using the key) say

std::unique_ptr<OtherType> get_othertype(std::string name)
{
   return otMap.find(name);
}

that would enure the caller would recieve a pointer to the one in the map rather than a copy? Is it ok to do this, or would it try and call the copy constructor (and fail as it has been removed) because it is being returned?

Assuming I must use unique_ptr as my map items.

UPDATE::

After trying to implement the code, it seems that unique_ptr and std:map/:pair dont work together in gcc 4.4.4, pair just didnt like unique_ptr as a type parameter. (see Can't create map of MoveConstructibles).

I changed the ptr to std::shared_ptr and it all worked.

I suppose I can use the same code with the shared pointer?

Solution 1

The model of unique_ptr is transfer of ownership. If you return a unique_ptr to an object from a function, then no other unique_ptr in the system can possibly refer to the same object.

Is that what you want? I highly doubt it. Of course, you could simply return a raw pointer:

OtherType* get_othertype(const std::string& name)
{
    return otMap.find(name)->second.get();
}

Thus, the client has access to the object, but the map still owns it.

The above solution is rather brittle in case there is no entry found under the name. A better solution would be to either throw an exception or return a null pointer in that case:

#include <stdexcept>

OtherType* get_othertype(const std::string& name)
{
    auto it = otMap.find(name);
    if (it == otMap.end()) throw std::invalid_argument("entry not found");
    return it->second.get();
}

OtherType* get_othertype(const std::string& name)
{
    auto it = otMap.find(name);
    return (it == otMap.end()) ? 0 : it->second.get();
}

And just for completeness, here is Anthony's suggestion of returning a reference:

OtherType& get_othertype(const std::string& name)
{
    auto it = otMap.find(name);
    if (it == otMap.end()) throw std::invalid_argument("entry not found");
    return *(it->second);
}

And here is how you return a reference to the unique_ptr inside the map, but let's make that a reference to const, so the client does not accidentally modify the original:

unique_ptr<OtherType> const& get_othertype(const std::string& name)
{
    auto it = otMap.find(name);
    if (it == otMap.end()) throw std::invalid_argument("entry not found");
    return it->second;
}

Solution 2

What is the type of otMap?

If otMap.find(name) returns a std::unique_ptr<OtherType> as an rvalue then this will work fine. However, ownership of the pointed-to value has now been transferred to the returned pointer, so the value will no longer be in the map. This would imply you were using a custom map type rather than std::map<>.

If you want to be able to have the value in the map and return a pointer to it, then you need to use std::shared_ptr<OtherType> both as the map value type and the return type of get_othertype().

std::map<std::string,std::shared_ptr<OtherType>> otMap;
std::shared_ptr<OtherType> get_othertype(std::string name)
{
    auto found=otMap.find(name);
    if(found!=otMap.end())
        return found->second;
    return std::shared_ptr<OtherType>();
}

Comments

[Mark]

I was following advice from another answer from here stackoverflow.com/questions/3759119/… where AshleysBrain suggested using unique_ptr to store in a collection

[Mark]

So what is the standard way to do this? I would want the callee to have a pointer to the objet but the map still be the owner. However I wouldnt want to use a raw as the callee might not dispose of it? What would happen then if I delete it fromt he map, his raw would be pointing to nothing?

[David Rodríguez - dribeas]

@Mark: Is the object guaranteed to be in the map? If so, then you can return a reference instead of a pointer and that way the semantics of the call are more explicit: I will give you access but not ownership.

[Mark]

The way the system is designed if the object wasnt there it would be an exception. The reference idea sounds like the best option.

[fredoverflow]

@Mark: The callee is not supposed to dispose of it. In fact, that would be a very bad idea, because the unique_ptr is responsible for disposal of the object! If the client holds on to the raw pointer after the unique_ptr has been destroyed and then dereferences the raw pointer, you get undefined behavior. Is the client supposed to store the pointer he gets from get_othertype somewhere? Do you want unique ownership or shared ownership? References don't solve this problem at all, the client can hold on the the referred object just as well if he wants to. Does he want to? is the question.

[Anthony Williams]

The overhead of shared_ptr over unique_ptr is small (a reference count). Using unique_ptr in this case requires careful thought, and I would strongly recommend against it unless it really does exactly what you want.

[Mark]

@FredOverflow: The caller (I had said callee before sorry) would just be manipulating the object not taking ownership, there is only one owner and that is the map. I am just thinking why the caller wouldnt use a smart pointer, why should he use raw pointers?

[fredoverflow]

@Mark: So the use case is foo.get_othertype("bar")->do_stuff_now() and not variable_for_later_use = foo.get_othertype("bar")? Then the raw pointer example code I posted in my answer is exactly what you want. Why would you use a smart pointer here? What benefits would it give you? What kind of smart pointer would you suggest? I can't think of a scenario that works without changing the underlying map structure from unique_ptr to shared_ptr, and I see little value in doing that.

[Mark]

YEah my mistake, but you get the jist I hope

[Mark]

@Fred: Well I might want to hold it as a local pointer for the scope of the caller function (but not more that that), rather then accessing it (If I have to make a few manipulations)

[fredoverflow]

@Ant: I disagree. unique_ptr clearly expresses the programmer's intent of the map owning the objects, and it makes the code potentially faster. A correct implementation of shared_ptr has to take thread-safety into account to get the reference counting right, which makes it potentially a lot slower than unique_ptr.

[fredoverflow]

@Mark: Storing a local copy is not problematic at all. The only thing you should not do is store the pointer in a global variable or somewhere else where it is accessible long after the unique_ptr has vanished.

[Mark]

I am fairly new to C++, just as a cavaet. I see now that the raw pointer returned would be stored in the stack frame of the caller and would pop at the end. I think I have got it now? So raw pointer returned is the best option.

[fredoverflow]

@Mark: Yes, and "popping" a local raw pointer at the end of a function has no effect whatsoever. Just for clarity, if a local unique_ptr is "popped", the object it refers to is destroyed immediately. And if a shared_ptr is "popped", the associated reference count is decremented by one, and if it has reached zero, the object is destroyed. And just to reassure you, given the use case that you described, returning a raw pointer seems to be the best option to me.

[Mark]

@Fred : Yes I agree, I will be using your solution exactly, thanks for your explanation it has helped me understand how to use unique_ptr properly.

[fredoverflow]

@Mark: Good. Now that we know what you need, let's make the solution safer. See my update.

[Anthony Williams]

Sure. Putting unique_ptr in the map says that the map owns the object. However, it leaves you with the problem of how to access it. If you access it directly e.g. with otMap[name]->something then that's OK, but returning a raw pointer is less desirable. The ref count overhead of shared_ptr is small unless the object is actually shared between threads. Small enough that I wouldn't worry unless profiling told me otherwise, anyway.

[Anthony Williams]

I would suggest that you return a reference rather than a raw pointer. It makes it clear that the map owns the object. Of course, in this case you'd better throw if the object isn't there.

[fredoverflow]

@Anthony: The client could still get() a raw pointer from the shared_ptr. C++ requires responsible usage, there is no 100% safety.

[Anthony Williams]

If they do that that's their problem. If you give them a raw pointer, that's your problem. I don't like APIs that give me raw pointers, as it's not clear who owns it, or what the lifetime is.

[fredoverflow]

@Anthony: I added example code that returns a reference. Feel free to edit according to your taste.

[Mark]

SO using a reference is preferred? Is that because its somewhat safer (ie you cant change it) and you cant assing null so we must throw and excpetion?

[fredoverflow]

@Mark: Preference is a very subjective matter :-) Anthony prefers the reference solution, I prefer the pointer solution. I suggest you read a good book on C++, learn about the differences between pointers and references, and then decide for yourself. By the way, you could also return a reference to the unique pointer in the map, let me just update my answer...

[Admin]

@Mark: Using a reference is preferred because it has tighter post-conditions: the function always returns a reference to a valid object in the map (or it raises an exception). It's simpler to use the object (slightly, no dereference) and easier to understand the ownership at a glance. (cont'd)

[Admin]

If you return a pointer, code that doesn't check for null pointers would "look wrong"—we eliminate that entire situation, replacing it with a model that tells you when something is wrong rather than silently doing the absolute wrong thing, which is continue with the null pointer as if it wasn't null.

[fredoverflow]

@Roger: And how can you be certain that there always is a valid object? Even if the name exists as a key inside the map, it could be mapped to an unbound unique_ptr. In that case, *(it->second) leads straight into undefined behavior land. Starting with a (smart) pointer and then ending up with a reference somehow feels wrong to me, but I guess this is an entirely subjective matter.

[Admin]

@Fred: If the map stores unique_ptrs, then the unique_ptr is the valid object in the map. However, if you want to additionally check the pointed-to object, that's certainly an option and would be additional semantics. Also note that some smart managers can't store nulls, such as Boost's pointer containers, so it depends what type of smart pointer you have.

[Mark]

Thanks guys, I have 3 options now , raw, ref, or ptr to unique_ptr. As you saw its a preference thing as long as the post conditions are checked for and the caller gets a valid mechanism to access the underlying object.

[sellibitze]

The point is unique ownership. That doesn't exclude the possibility of other raw pointers pointing to the same object. In that sense it's not much different compared to a container that returns an iterator.

[scry]

@Fred: Correct me if I'm wrong, but it seems that attempting to actually use the last version (for anything other than an rvalue) will fail, because it requires the use of unique_ptr(const unique_ptr&), which is a deleted function.

[fredoverflow]

@roysc The unique_ptr is neither copied nor moved out of the function. Note the const& in the return type! The original is simply passed back by const reference.

[scry]

@FredOverflow Though that makes sense to me, but this code fails to compile. Apparently it does use the copy constructor, but why?

[fredoverflow]

@roysc In line 23, auto r = ... makes a copy, even if ... is returned by reference. Try auto& r instead.