Best way to append object to a vector of smart pointers?

Question

Considering the following classes:

class A {
 public:
    A() {};
 protected:
    int m1;
    float m2;
    // ...
}

class B {
 public:
    B() { };
    void add1(std::shared_ptr<A> _obj) { 
        data.push_back(_obj); 
    };
    void add2(A* _obj) { 
        data.push_back(_obj); 
    };
    void add3(A& _obj) { 
        data.push_back(&_obj); 
    };
 private:
    std::vector<std::shared_ptr<A>> data;
}

which of the methods addX in class B would be more suited/best practice for passing a new pointer into the vector? Considering that data is a vector of non-const smart pointers, I understand it is not possible to pass the object or the smart pointer by const reference without having to "de-const" them which is not advisable and considered bad practice. However, unless I am mistaken, all those methods end up instantiating a shared_ptr and then copying it into the vector? What are the differences between them?

All 3 have different meaning, and 3rd is actually wrong (I can't imagine any not dangerous use-case for it). You have to consider ownership of the objects when choosing the method. Which party should hold the ownership of the object? Caller of add? B? Both? — Yksisarvinen
In this case, ownership is, by design, supposed to be shared, thus why I am using shared_ptr. Instances of A are however, are assumed to be instantiated outside the scope of B, but after appended to the member vector, B can do whatever with it. — joaocandre
Option 2 is not using shared ownership. You expect caller to pass the ownership to B, because B will delete the pointer. And you need to make that very clear in documentation. — Yksisarvinen
Note that you could also pass a std::unique_ptr<A> or a std::weak_ptr<A>. add1 would accept either — Caleth
@Caleth would there any advantages over a plain shared_ptr? Is unique_ptr for instance smaller and thus simpler to move? — joaocandre

Vittorio Romeo Vittorio Romeo · Accepted Answer · 2019-07-12T13:52:54

If you want to ensure that your API makes it clear that the callee is going to take ownership, the only viable method is add1. It makes it clear from the signature that whatever you give it is going to be retained by the callee.

You can improve it by using std::move and .emplace_back:

void add1(std::shared_ptr<A> _obj) 
{ 
    data.emplace_back(std::move(_obj)); 
}

add2 is misleading, as a raw pointer might or might not imply ownership. Someone could pass a pointer to a stack-allocated variable.

add3 has the same problem, but it is even more misleading as non-const references are commonly used for mutating an object in-place, not taking ownership of it.

Best way to append object to a vector of smart pointers?

2 Answers