19

Can't find much on that for C++11 but only on boost.

Consider the following class:

class State
{
   std::shared_ptr<Graph> _graph;

 public:

    State( const State & state )
    {
        // This is assignment, and thus points to same object
        this->_graph = std::make_shared<Graph>( state._graph ); 

        // Deep copy state._graph to this->_graph ?
        this->_graph = std::shared_ptr<Graph>( new Graph( *( state._graph.get() ) ) );

        // Or use make_shared?
        this->_graph = std::make_shared<Graph>( Graph( *( state._graph.get() ) ) );
    }   
};

Suppose class Graph does have a copy constructor:

Graph( const Graph & graph )

I do not want to have this->_graph point/share the same object! Instead, I want this->_graph to deep copy the object from state._graph, into my own this->_graph duplicate.

Is the way above the correct way of going about this?

Documentation of std::make_shared notes that:

Moreover, f(shared_ptr(new int(42)), g()) can lead to memory leak if g throws an exception. This problem doesn't exist if make_shared is used.

Is there another way of going about this, safer or more reliable?

Ælex
  • 14,432
  • 20
  • 88
  • 129
  • Is there a reason you're using a `shared_ptr` if you want each object to point to its own `Graph`? It seems like another pointer type would be more appropriate, as would having the `Graph` as a direct subobject. – templatetypedef Nov 14 '13 at 04:06
  • Yes, they are owned between other classes. But in this particular case I really need a duplicate, and not to share the given Object as I intend to alter it, without wanting the original to be altered. You propose I use unique_ptr instead? – Ælex Nov 14 '13 at 04:10
  • That was going to be one of my suggestions, but if the object really is shared then there are lots of other good approaches. – templatetypedef Nov 14 '13 at 04:16

1 Answers1

12

If you want to make a copy of the Graph object when you make a copy of the object, you can always define your copy constructor and assignment operator to do just that:

State::State(const State& rhs) : _graph(std::make_shared(*rhs._graph)) {
   // Handled by initializer list
}
State::State(State&& rhs) : _graph(std::move(rhs._graph)) {
   // Handled by initializer list
}
State& State::operator= (State rhs) {
    std::swap(*this, rhs);
    return *this;
}

Hope this helps!

templatetypedef
  • 362,284
  • 104
  • 897
  • 1,065
  • I'm even more curious now. The first Copy constructor you have, simply assigns the same graph, does it not? Whereas the second one uses move, which would move, not duplicate the object, no? – Ælex Nov 14 '13 at 04:21
  • 2
    The copy constructor initializes `_graph` to be a `std::shared_ptr` that points at a brand-new `Graph` object initialized as a copy of the original graph. This means that it ends up pointing to a new `Graph` separate from the original graph. In the move constructor, we just move the existing `shared_ptr` out of the existing `State` object, since that object isn't going to be used any more. – templatetypedef Nov 14 '13 at 04:28
  • 4
    Additional explanation: `_graph = std::make_shared( * state._graph );` _(note the asterisk!)_ will call the copy ctor of `Graph` because `make_shared` takes arguments to pass to a ctor, and `*state._graph` gives a reference to the object managed by the shared_ptr. A ctor that takes a reference of its own type would be the copy-ctor (assuming there is one). If all you wanted was to share ownership with another shared_ptr then it would simply be `_graph(state._graph)` (copy-ctor of shared_ptr). You only need `make_shared` when you want **a new `T` object** (wrapped in a shared_ptr). – AnorZaken Aug 30 '16 at 15:17