Should I use std::shared pointer to pass a pointer?

c++, c++11, pointers, smart-pointers

Solution

If the ownership of the managed object is not being transferred (and because it's a `unique_ptr`, ownership cannot be shared) then it's more correct to separate the logic in the called function from the concept of ownership. We do this by calling by reference.

This is a convoluted way of saying:

Given:

std::unique_ptr<Thing> thing_ptr;

to change the Thing:

// declaration
void doSomethingWith(Thing& thing); 

// called like this
doSomethingWith(*thing_ptr);

to use the Thing without modifying it.

// declaration
void doSomethingWith(const Thing& thing); 

// called like this
doSomethingWith(*thing_ptr);

The only time you'd want to mention the `unique_ptr` in the function signature would be if you were transferring ownership:

// declaration
void takeMyThing(std::unique_ptr<Thing> p);

// call site
takeMyThing(std::move(thing_ptr));

You never need to do this:

void useMyThing(const std::unique_ptr<Thing>& p);

The reason that this would be a bad idea is that if confuses the logic of useMyThing with the concept of ownership, thus narrowing the scope for re-use.

Consider:

useMyThing(const Thing& thing);

Thing x;
std::unique_ptr<Thing> thing_ptr = makeAThing();
useMyThing(x);
useMyThing(*thing_ptr);

Update:

Noting the update to the question - storing (non-owning) references to this object.

One way to do this is indeed to store a pointer. However, pointers suffer from the possibility of a logic error in that they can legally be null. Another problem with pointers is that they do not play nicely with `std:: algorithms` and containers - requiring custom compare functions and the like.

There is a `std::-compliant` way to do this - the `std::reference_wrapper<>`

So rather than this:

std::vector<Thing*> my_thing_ptrs;

do this:

std::vector<std::reference_wrapper<Thing>> my_thing_refs;

Since `std::reference_wrapper<T>` defines an operator `T&`, you can use the `reference_wrapped` object in any expression that would expect a `T`.

for example:

std::unique_ptr<Thing> t1 = make_thing();
std::unique_ptr<Thing> t2 = make_thing();
std::unique_ptr<Thing> t3 = make_thing();

std::vector<std::reference_wrapper<const Thing>> thing_cache;

store_thing(*t1);
store_thing(*t2);
store_thing(*t3);

int total = 0;
for(const auto& t : thing_cache) {
  total += value_of_thing(t);
}

where:

void store_thing(const Thing& t) {
  thing_cache.push_back(std::cref(t));
}

int value_of_thing(const Thing& t) {
  return <some calculation on t>;
}

Problem

Suppose I have an object which is managed by an `std::unique_ptr`. Other parts of my code need to access this object. What is the right solution to pass the pointer? Should I just pass the plain pointer by `std::unique_ptr::get` or should I use and pass an `std::shared_ptr` instead of the `std::unique_ptr` at all? I have some preference for the `std::unique_ptr` because the owner of that pointer is actually responsible for cleanup. If I use a shared pointer, there's a chance that the object will remain alive due to a shared pointer even when it should actually be destroyed. EDIT: Unfortunately, I forgot to mention that the pointer will not just be a an argument to a function call, but it will be stored in other objects to build up a network structure of objects. I do not prefer the shared pointer because then it's no longer clear, who actually owns the object.

Original source