Should I use unique_ptr to keep class' members?

Question

I have such code:

class A
{
public:
    A(void);
    ~A(void)
    {
        delete b;
        delete c;
        delete d;
        // ...
    }
private:
    B* b;
    C* c;
    D* d;
    // ...
};

//A.cpp
    A(void) : b(new B()), c(new C()), d(new D()) //...
    {  
    }

Class A takes ownership over own objects b, c, d ... What is the best way to keep these objects? I guess, that usage of std::unique_ptr<B/C/D> type will be suitable for this way. For example, it allows to don't care about carefull writing of destructor.

Yes, class A does not have any information about exact sizes of B, C, D — LmTinyToon, May 25 '16 at 14:08
@АлександрЛысенко: Then, you should not put the definition of the constructor in the *header*, as you have done here. If you can do `new B`, then you have information about the exact sizes of B. — Nicol Bolas, May 25 '16 at 14:09
@NathanOliver: There are good reasons not to do that. But if he's not going to, then he can't allocate them in the header either. — Nicol Bolas, May 25 '16 at 14:10
yes, you are right, I have implementation of A() method in cpp file — LmTinyToon, May 25 '16 at 14:10
@АлександрЛысенко: I wasn't just talking about the destructor. You can't call `new B` unless you have the definition of `B`. And if you do have the definition, you could just make them values rather than pointers. Lastly, how do you define "the best way to keep these objects"? — Nicol Bolas, May 25 '16 at 14:12
By all means, use `std::unique_ptr` or `std::shared_ptr` as appropriate for this sort of thing. It makes you code safer - even from issues you don't realize you have. But, frankly, if you're _always_ allocating these objects in your constructor then you almost certainly should not use pointers. You may even be able to avoid pointers even if you need to pass instances of constructed objects into the constructor - simply use move semantics to move the object in. — Nik Bougalis, May 25 '16 at 14:15
Unrelated and not really a problem: In C++, unlike in C, a `(void)` parameter list is equivalent to `()`. C++ programmers usually prefer `()`. — molbdnilo, May 25 '16 at 14:22

score 6 · Accepted Answer · edited May 23 '17 at 12:24

it allows to don't care about carefull writing of destructor.

More than that.

Your code is not exception-safe. For example, if new D() failed by exception being thrown, delete b and delete c won't be executed and memory will leak, because destructor won't be called if constructor fails. Smart pointers can help you to avoid this kind of situation.
Holding raw pointer as members you need to implement destructor carefully, and copy constructor and assignment etc too. See What is The Rule of Three? and Rule-of-Three becomes Rule-of-Five with C++11?.

bobah · Answer 2 · 2016-05-25T14:17:27.117

Best is to keep everything by value. If it fits*, and does not need to be hidden**. If it does not fit or needs to be hidden first preference is std::unique_ptr***, second preference (if ownership has to be shared) is std::shared_ptr. And only as a last resort (example for which I cannot even think up). You would actually have raw pointers and manage lifetime yourself, with risk of memory errors and leaks.

* - sometimes you want to be able to have parent object on stack by value and child objects are, say, large arrays which, if stored by value would overflow the stack

** - sometimes you don't want to show what child objects really are (because they are complex, say boost.fusion adapted classes. Then you would want some form of PIMPL idiom:

class.hpp
struct b;
struct A { std::unique_ptr<b> b_; A(); ~A(); }

class.cpp:
struct b { ... }
A::A() = default;
A::~A() = default;

*** - automatic management of dynamically allocated members with unique_ptr

struct A {
  std::unique_ptr<b> b_;
  A(...):
    b_(std::make_unique<b>(...)) {}
};

score 2 · Answer 3 · answered May 25 '16 at 14:36

2

I think it's worth mentioning that if you do not want to transfer ownership, you must use const std::unique_ptr. Using a non-const std:unique_ptr allows to transfer it to another std:unique_ptr.

answered May 25 '16 at 14:36

perencia

1,398
1
11
16

Should I use unique_ptr to keep class' members?

3 Answers3