16
votes

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?

1
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
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
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

1 Answers

9
votes

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!