Proper Implementation of Copy Constructor and Equals Operator on a class with smart pointers

Question

Suppose I want to implement a class which is copyable, so I can implement the copy constructor and assignment operator. However, what is the correct implementation and handling of unique and shared pointer variables? See this contrived example which has both types of pointers:

Header File

#include <memory>

using std::unique_ptr;
using std::shared_ptr;

class Copyable
{
private:
    unique_ptr<int> uniquePointer;
    shared_ptr<int> sharedPointer;

public:
    Copyable();
    Copyable(int value);
    Copyable(const Copyable& other);
    ~Copyable();

public:
    int GetUniqueValue() { return *uniquePointer; };
    int GetSharedValue() { return *sharedPointer; };
    Copyable& operator=(const Copyable& other);
};

CPP File

#include "stdafx.h"
#include "Copyable.h"

using namespace std;

Copyable::Copyable() : 
    uniquePointer(make_unique<int>()), sharedPointer(make_shared<int>())
{
}

Copyable::Copyable(int value) : 
    uniquePointer(make_unique<int>(value)), 
    sharedPointer(make_shared<int>(value))
{
}

Copyable::Copyable(const Copyable& other) : 
    uniquePointer(make_unique<int>(*other.uniquePointer)), 
    sharedPointer(make_shared<int>(*other.sharedPointer))
    // OR
    sharedPointer(other.sharedPointer)
{
}

Copyable::~Copyable()
{
}

Copyable& Copyable::operator=(const Copyable& other)
{
    if (&other != this)
    {
        uniquePointer.reset();
        uniquePointer = make_unique<int>(*other.uniquePointer);

        sharedPointer = make_shared<int>(*other.sharedPointer);
        // OR
        sharedPointer = other.sharedPointer;
    }

    return *this;
}

Usage Allows Copying

Copyable copyable1(5);
int uniqueValue1 = copyable1.GetUniqueValue();
int sharedValue1 = copyable1.GetSharedValue();
Copyable copyable2 = copyable1;
int uniqueValue2 = copyable2.GetSharedValue();
int sharedValue2 = copyable2.GetSharedValue();

There is only one way to copy the unique pointer using the make_unique function but what about the shared pointer? Should I assign it or use the make_shared function?

UPDATE - Copying versus Moving

One a wider note I'm trying to figure out when to use what. If I decide to use copying, why would I use unique_ptr? It seems shared_ptr is the way to go. Equally, if using move semantics, unique_ptr seems the way to go. Generally speaking only. I should perhaps split this into a separate question.

No, you ***can't*** copy a unique pointer, otherwise it wouldn't be *unique*. — Some programmer dude, Jul 22 '14 at 14:35
But you can copy the contents of it no? Is this bad practice then? — Muhammad Rehan Saeed, Jul 22 '14 at 14:36
Yes, you can copy *what it points to*, but not the actual pointer. Also, when you invoke the [`std::make_unique`](http://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique) function it will call the [`std::unique_ptr`](http://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique) [constructor](http://en.cppreference.com/w/cpp/memory/unique_ptr/unique_ptr), and the constructor taking another `std::unique_ptr` object *transfers* the *ownership* of the pointer. And that last sentence should give you a hint about how you should look at the new smart pointers, namely in terms of ownership. — Some programmer dude, Jul 22 '14 at 14:39
The entire point of a `unique_ptr` is that that one particular `unique_ptr` is the *only* pointer to a given object. When the unique_ptr is destroyed, then it will destroy the object. So if you somehow were to copy the value of the `unique_ptr` to another `unique_ptr`, then when either of them is destroyed, the pointed-to object is destroyed, and now one of the `unique_ptr`s is pointing to freed memory and you'll probably cause a use-after-free. — antiduh, Jul 22 '14 at 14:39
Do you mean something like unique_ptr uniquePointer2 = make_unique(uniquePointer) transfers ownership? — Muhammad Rehan Saeed, Jul 22 '14 at 14:41
Ah, thank you @antiduh. So as a general rule, if I am using a unique_ptr, always disallow copying and use move semantics? Is that correct? — Muhammad Rehan Saeed, Jul 22 '14 at 14:43
@RehanSaeed it's the other way round; if you want move-only semantics for your class you can use a `unique_ptr` member; otherwise, you should use a different pointer facility. — ecatmur, Jul 22 '14 at 14:47
I'm trying to figure out when to use what. If I decide to use copying, why would I use unique_ptr? It seems shared_ptr is the way to go. Equally, if using move semantics, unique_ptr seems the way to go. Generally speaking only. — Muhammad Rehan Saeed, Jul 22 '14 at 14:50

Niall · Accepted Answer · 2014-07-22T14:49:02.020

1

What about the shared pointer? Should I assign it or use the make_shared function?

tl;dr The assignment is most likely what you are looking for.

This depends entirely on the semantics of the class involved;

If you want the objects to share the state of the shared_ptr then an assignment or a copy would be required
If you want each object to maintain it's own state, then create a new shared_ptr based on the "other" object

If the shared state it not required, you really are just better off with a unique_ptr

As a general "rule of thumb";

If your type contains move only members, then only allow moving. If your type has copyable members, allow copying. Where it makes sense, follow the "value" type semantics. Strive for the "rule of zero"

edited Jul 22 '14 at 14:49

answered Jul 22 '14 at 14:42

Niall

30,036
10
99
142

I see, so assignment should be used for the shared_ptr in my case. But what about copying vs moving. As a general rule, I think I now understand that if using unique_ptr, use move semantics and disallow copying. What about shared_ptr, is there a general rule I can apply? – Muhammad Rehan Saeed Jul 22 '14 at 14:47
1

@RehanSaeed Copy semantics. Basically `shared_ptr` implies shared state with the copy semantics being; all the copies share the same object being managed by the `share_ptr` – Niall Jul 22 '14 at 14:51

Proper Implementation of Copy Constructor and Equals Operator on a class with smart pointers

1 Answers1

Linked