1

I have a user defined class that has a std::unique_ptr member. I am trying to overload the assignment operator to new an object of the same type to the unique_ptr member or assign the value to the ptr value.

class object
{
    public:
    // POD types

    object& operator=(const object& _obj);

    std::unique_ptr<baseClass> ptr;
} 

I've tried using:

std::unique_ptr::swap()

but the swap function is non-const so I attempted assigning the the result of:

std::unique_ptr::get()

to the ptr and then calling:

std::unique_ptr::release()

but release also is non const, so I cannot guarantee that ptr will be destructed correctly since the old ptr will still have ownership. Alas, operator=() for unique_ptr does not accept a non-const reference to another unique_ptr so I have opted to make the operator= as so for my class.

object& object::operator=(const object& _obj)
{
    //POD types use operator= as normal
    ptr.reset(nullptr);

    return *this;
}

And just call a setPtr() method later on to set the ptr. I wanted to just detect the type and just assign the ptr using:

ptr.reset(new detectedType );

but the consensus all over Stack Overflow and my preferred search engine is that detecting types like this would be a design flaw. My question is: am I overlooking something simple and is there a better way to overload the assignment operator for user defined types that have a unique_ptr member?

Before post edit: I changed the parameter to object& rather than const object& and it worked. Is this a good way to correct this issue or should I be trying to work with only const references to the other class?

Haatschii
  • 9,021
  • 10
  • 58
  • 95
Nathan Krone
  • 63
  • 1
  • 9

2 Answers2

1

My question is: am I overlooking something simple and is there a better way to overload the assignment operator for user defined types that have a unique_ptr member?

There is indeed. This breaks down to understanding what smart pointers in C++ do. Usually you use a std::unique_ptr if there will be one and only one reference to your object. This is the reason why std_unique_ptrs are neither copy-constructable nor copy-assignable. If you manually create two copies of a std::unique_ptr as you describe in you post by using ptr.get() you will end up in an invalid state, because as soon as one copy leaves scope, the std::unique_ptr destructor will also destruct the object it points to, leaving the other std::unqiue_ptr in an invalid state (it points to a freed memory location). So if there might be several references to your object you should probably use std::shared_ptr instead.

In your examples the question is what do you want to achieve?

If you want your assignment operator such that after calling a = b, a.ptr and b.ptr point to the exact same object (i.e. store the same pointer) use a shared pointer. You should definitely NOT use a unique pointer in this case, as calling the destructor for a or b would destruct the object pointed to, leaving a or b in an invalid state.

If you want that after calling a = b, a.ptr points to independent copy of *(b.ptr) you can indeed use a unique pointer and implement you assignment operator like this (provided baseClass is copy construable) :

object& object::operator=(const object& _obj)
{
    //POD types use operator= as normal
    ptr.reset(new baseClass(*_obj.ptr));

    return *this;
}

Note however that this only works if baseClass is final (unlikely considering the name...), i.e. ptr won't store pointers to objects of a derived class. Otherwise this results in slicing as pointed out by BenVoigt.

Finally, it is generally expected that calling a = b does not change b. Therefore making the ptr member mutable and using swap sounds like a bad idea to me. If you want to achieve this behavior because of performance (or other) considerations, I would suggest to implement the move assignment operator instead

object& object::operator=(object&& _obj)
{
    //POD types use operator= as normal
    ptr = std::move(_obj.ptr);

    return *this;
}

and call a = std::move(b), which makes clear that b might end up in a different state.

Haatschii
  • 9,021
  • 10
  • 58
  • 95
  • Unfortunately `new baseClass(*_obj.ptr)` suffers from slicing. One needs a polymorphic "clone" system to make a copy which has the correct runtime type. – Ben Voigt Oct 24 '18 at 05:37
  • @BenVoigt Correct. I didn't quite think about the implications of the name `baseClass`. I updated my answer to mention this problem. – Haatschii Oct 24 '18 at 13:18
0

This is a good use case for the mutable keyword. If you declare ptr as mutable, you are indicating that even when an instance of your class is semantically const, it must perform non-const operations on a particular data member. Declare your class as follows, and you should be able to implement your original idea of using swap:

class object
{
    public:
    // POD types

    object& operator=(const object& _obj);

    mutable std::unique_ptr<baseClass> ptr;
} 
Derek T. Jones
  • 1,800
  • 10
  • 18
  • 1
    This of course depends on the purpose of `ptr`, in general however I would strongly argue against this approach, because you would generally expect that `a=b` does **not** change `b`. See my answer for details. – Haatschii Oct 24 '18 at 04:57
  • `mutable` will get this code past the compiler, but it still won't make it right. C++ deprecated silent-ownership-stealing `unique_ptr` (it was called `auto_ptr` at the time) for a good reason. – Ben Voigt Oct 24 '18 at 05:40