2

I've learnt that when you use pointers in a class, you should implement the rule of 5. If you do not use pointers then you're okay, and in fact its preferable, to use the defaults. However, how does this work with smart pointers? For example a class containing an int* might look like this:

class A {
private:
    int *num_;
public:

    explicit A(int* num) : num_(num) {}

    ~A() {
        delete num_;
    }

    A(const A &other) {
        if (this != &other) {
            num_ = other.num_;
        }
    }

    A(A &&other) noexcept {
        if (this != &other) {
            num_ = other.num_;
        }
    }

    A &operator=(A &other) {
        if (this == &other) {
            this->num_ = other.num_;
        }
        return *this;
    }

    A &operator=(A &&other) noexcept {
        if (this == &other) {
            this->num_ = other.num_;
        }
        return *this;
    };


};

But if we use smart pointers, is it sufficient to just do this?

class B {
private:
    std::unique_ptr<int> num_;

public:

    explicit B(int num) : num_(std::make_unique<int>(num)) {};

};
CiaranWelsh
  • 7,014
  • 10
  • 53
  • 106
  • 2
    Mind you that the examples don't do the same thing, as the second one will have implicitly deleted copy constructor and assignment operator, as `std::unique_ptr` is not copyable – UnholySheep Apr 15 '20 at 08:02
  • Ah this makes sense. So should I have used a `std::shared_ptr`? – CiaranWelsh Apr 15 '20 at 08:05
  • It depends on if you want `B` to be copyable (both `A` and `B` are moveable) – Caleth Apr 15 '20 at 08:07
  • @CiaranWelsh that shouldn't be your rationale for chosing the type of smart pointer. Rather: do you need shared ownership or not. In `A` there is no shared ownership (at least not correctly ;) – 463035818_is_not_an_ai Apr 15 '20 at 08:07
  • Note that your `A` class implements a copy constructor which actually copies the pointer. So you'll have two `A` objects sharing the same pointer, which will attempt to delete it twice on destruction. `unique_ptr` prevents this by deleting the copy constructor (and copy assignment). I'm unsure about what kind of smart pointer you want to implement -- `A` looks unsafe to be used. – chi Apr 15 '20 at 08:09
  • 1
    I suppose this is a typo: `if (this == &other) {` – 463035818_is_not_an_ai Apr 15 '20 at 08:20
  • So to summarize, `A` is unsafe because it copies the `num_` pointer not value; I should decide whether I need shared ownership of `A` instead of using a `shared_ptr` to prevent the compiler from implicitely deleting copy and assignment special members and `if (this == & other) {` isn't the correct way to protect against self copying/assignment. Looks like I have some work to do - thanks for all the advice! – CiaranWelsh Apr 15 '20 at 08:36

3 Answers3

4

Yes that is sufficent. The unique pointer does manage the memory. However, your two classes will behave differently because std::unique_ptr cannot be copied, hence there will be no compiler generated copy constructor nor assignment for B.

Also note that you implemented all methods for the rule of 5, but not correctly. As mentioned in a comment, copying an A will result in two instances having the same pointer and delete it upon destruction. Actually getting this right is the whole point about the rule of 3/5 and why you should prefer the rule of 0.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
2

Those have different behaviour. A can be copied, B can only be moved.

N.B. your implementation of A is unsafe, it can lead to leaks and undefined behaviour.

A comparison of like-for-like would either delete A's copy

class A {
private:
    int *num_;
public:

    explicit A(int num) : num_(new int(num)) {}

    ~A() {
        delete num_;
    }

    A(const A &other) = delete;

    A(A &&other) noexcept 
     : num_(std::exchange(other.num, nullptr)) {}

    A &operator=(const A &other) =delete;

    A &operator=(A &&other) noexcept {
        swap(num_, other.num_);
        return *this;
    };
};

class B {
private:
    std::unique_ptr<int> num_;

public:

    explicit B(int num) : num_(std::make_unique<int>(num)) {};

};

Or define B's copy

class A {
private:
    int *num_;
public:

    explicit A(int num) : num_(new int(num)) {}

    ~A() {
        delete num_;
    }

    A(const A &other) 
     : A(other.num) {}

    A(A &&other) noexcept 
     : num_(std::exchange(other.num, nullptr)) {}

    A &operator=(const A &other) {
        *num_ = *other.num;
        return *this;
    }

    A &operator=(A &&other) noexcept {
        swap(num_, other.num_);
        return *this;
    };
};

class B {
private:
    std::unique_ptr<int> num_;

public:

    explicit B(int num) : num_(std::make_unique<int>(num)) {};
    ~B() = default;
    B(const B & other) : B(*other.num_) {}
    B(B && other) = default;
    B& operator=(const B & other) { *num_ = *other.num_ }
    B& operator=(B && other) = default;

};
Caleth
  • 52,200
  • 2
  • 44
  • 75
1

If you use smart pointer (or any of the std:: containers) the class default destructor will call the destructor of the smart pointer (and containers). More on this topic here: Why doesn't the C++ default destructor destroy my objects?

Jean-Marc Volle
  • 3,113
  • 1
  • 16
  • 20