Should I use smart pointers here?

Question

I have read several answers about similar issues, but I am still confused as to when it is a good time to use smart pointers. I have a class Foo which looks like this:

class Bar;
class Baz;

class Foo
{
public:
    Foo(Bar* param1, std::vector<Baz*>& param2);

    virtual ~Foo();

    // Method using myBar and myBaz polymorphically...

private:
    Bar*               myBar;
    std::vector<Baz*>  myBaz;
};

I need the two data members to be pointer for polymorphism. It is part of an API and what I fear is that someone writes:

int main()
{
    //...

    std::vector<Baz*> allOnHeap {new Baz(), new Baz()};

    Foo(new Bar(), allOnHeap);

    // ...
}

which would be legal but would result in memory leaks. I could add deletes in the destructor, but then what if no dynamic allocation has been made? In your opinion, would it be a good idea to use smart pointers in this case? Please explain your opinion. If you have better ideas on how to do this, please feel free to share it.

Smart pointers are all about ownership. This issue comes down to whether you want `Foo` to own the objects it points to. If no, then your code is fine, and the user of `Foo` is required to manage the object lifetimes. If they leak, so be it, not `Foo`'s fault. But, if you want `Foo` to have ownership, then change `myBar` to `std::unique_ptr` or `std::shared_ptr`, and change `myBaz` to `std::vector>` or `std::vector>`. Then there is never any question about who owns what. — Remy Lebeau, Sep 20 '17 at 00:24
Yes, this is one of the standard cases where `std::unique_ptr`s are useful. — Ken Y-N, Sep 20 '17 at 00:24
@RemyLebeau In which kind of context could I not care about the fact that my class could produce memory leaks? It seems to me that raw pointer data members kind of always lead to this kind of problem. Am I wrong? — BobMorane, Sep 20 '17 at 00:32
@BobMorane again, it's a matter of ownership. `Foo` does not allocate the objects. If it doesn't take ownership of the objects it is given, it has no business using smart pointers to them (or, at least, it can use shared ownership via `std::shared_ptr`). You already stated a context where raw pointers are desirable - "*no dynamic allocation has been made*" — Remy Lebeau, Sep 20 '17 at 00:35

score 1 · Answer 1 · answered Sep 20 '17 at 00:32

Smart pointers don't immediately jump to mind in this case mostly because it's not clear what the ownership requirements are. If you are following the C++ Core Guidelines recommendation on ownership transfer and raw pointers then it's clear that class Foo is not intended to take ownership of the Bar and Baz objects and should not be deleting them.

If your intent is to maybe sometimes take ownership of the objects and sometimes not to take ownership, I would suggest considering an alternate design where you are able to pick a single functionality and stick with it (:

Perhaps smart pointers should always jump to mind to define what the ownership semantics should be. — Werner Erasmus, Sep 20 '17 at 10:34

Werner Erasmus · Accepted Answer · 2017-09-20T14:27:39.183

I have read several answers about similar issues, but I am still confused as to when it is a good time to use smart pointers. I have a class Foo which looks like this:

I would use smart pointers in the case you've mentioned.

Presently copying of your class is not safe
You can't know whether the objects that your are referencing in the list still exists at the time of referencing it
RAII is not automatically used to for myBar

More specifically, one can IMHO model the ownership semantics that you require:

Use unique_ptr<Bar> and use std::vector<weak_ptr<Baz>>.

unique_ptr<Bar> gives you automatic cleanup at the end of the holding class's scope
std::vector<weak_ptr<Baz>>: Each of the weak_ptr's indicate that the using class (class that would get access through the weak_ptr) has no ownership semantics, and prior to usage has to attempt gaining temporary ownership (by locking which gives one a shared_ptr).

With the above mentioned usage of smart pointers, you will get a compiler error when mistakenly copying out of the box (as unique_ptr is not copyable), and more importantly, each item in the list will be safely accessible. The instantiator of the holding class also has a clear idea wrt the ownership semantics envisaged by the class writer

BTW, you don't have to explicitly delete your default constructor. See for detail.

Should I use smart pointers here?

2 Answers2