How should I construct an instance intended to be managed with std::shared_ptr?

Question

Consider the following parent/child object model. The intention is for both the parent and the child to use shared_ptr to manage their lifetimes. The parent should keep a shared_ptr to (retain) its children, and the children will keep a weak_ptr to the parent.

Given that the intention is for these objects to always be managed by std::shared_ptr what is the best way to construct them? The method I've come up with (so far) feels a little clunky: I'm using factory (friend) functions and a private constructor to reduce the likelihood that raw pointers to these objects "escape". Then, the child creates a shared_ptr with this in the ctor, and puts that into the parent's vector of children. When the ctor returns the raw pointer to the factory function, the factory function uses shared_from_this() to get a shared_ptr that is "hooked up to" (i.e. sharing a reference count with) the shared_ptr that's in the parent's vector of children.

Here's what I've come up with so far:

class Child; // Forward decl class Parent : std::enable_shared_from_this<Parent> { public: int value() const { return _value; }; void set_value(int newValue) { _value = newValue; }; std::vector<std::shared_ptr<const Child>> children() const { // propagate const-ness to the child pointers we hand back. return std::vector<std::shared_ptr<const Child>>(begin(_children), end(_children)); }; std::vector<std::shared_ptr<Child>> children() { return _children; }; private: Parent(int value) : _value(value) {}; friend class Child; // So it can add itself to the _children vector friend class std::shared_ptr<Parent>; // So that I don't have to inherit public from enable_shared_from_this friend std::shared_ptr<Parent> CreateParent(int value); // So it can call the private ctor std::vector<std::shared_ptr<Child>> _children; int _value; }; class Child : std::enable_shared_from_this<Child> { public: int value() const { return _value; }; void set_value(int newValue) { _value = newValue; }; private: Child(const std::shared_ptr<Parent>& parent, int value) : _parent(parent), _value(value) { std::shared_ptr<Child> sp(this); // This feels wrong, but the existence of the shared_ptr in the parent's vector of children ensures that the precondition for calling shared_from_this() is met parent->_children.push_back(sp); }; friend std::shared_ptr<Child> CreateChild(const std::shared_ptr<Parent>& parent, int value); // So it cal call the private ctor friend class std::shared_ptr<Child>; // So that I don't have to inherit public from enable_shared_from_this std::weak_ptr<Parent> _parent; int _value; }; std::shared_ptr<Parent> CreateParent(int value) { return std::shared_ptr<Parent>(new Parent(value)); }; std::shared_ptr<Child> CreateChild(const std::shared_ptr<Parent>& parent, int value) { std::shared_ptr<Child> rv = (new Child(parent, value))->shared_from_this(); return rv; };

This seems to work, but man, does it feel clunky. Is there a better way?

You skip on the details of the original problem you are trying to solve. Why would raw pointers "escape"? Is this a 'social' contract/constraint you are trying to address? — mockinterface
– mockinterface, Commented Apr 7, 2014 at 12:55
The original problem I'm trying to solve is my lack of a thorough understanding of best practices with smart pointers. Yes, the example is contrived, but that's intentional. — ipmcc
– ipmcc, Commented Apr 7, 2014 at 13:05
Oh. That's just kinda the state the code was in when I asked. That creator could just as well have been a static member function. I'm just partial to free functions, that's all. I guess to restate/narrow my confusion: In the case of Child, it feels like I'm "splitting" the creation of the shared pointer across the ctor and the factory method. Parent, on the other hand, it's clear, and in one place, because there's no other shared ownership. That splitting feels clunky. — ipmcc
– ipmcc, Commented Apr 7, 2014 at 13:16
To avoid the split don't generate the sp for the child in the ctor, it ain't best practice anyway (the creation hasn't finished), instead manage the child sp, and the (weak) relationship to the parent solely in the CreateChild method. — mockinterface
– mockinterface, Commented Apr 7, 2014 at 13:21

John Zwinck · Accepted Answer · 2014-04-07 13:23:30Z

I'd do it this way:

class Child; class Parent { public: std::vector<const Child*> children() const { // propagate const-ness to the child pointers we hand back. // ... } const std::vector<std::unique_ptr<Child>>& children() { return _children; } std::shared_ptr<Parent> create() { // I'd rather use std::make_shared() here but it needs public ctor return std::shared_ptr<Parent>(new Parent()); } std::unique_ptr<Child>& createChild() { _children.emplace_back(new Child(this)); return _children.back(); } private: Parent(); std::vector<std::unique_ptr<Child>> _children; }; class Child { private: Child(Parent* parent) : _parent(parent) {} friend class Parent; Parent* _parent; };

I stripped out the "value" boilerplate for clarity. I also tried to use the minimal sophistication level of smart pointers that seemed viable to get the job done. I may not have gotten it 100% perfect for your specific use case, but you should think about more clearly defined lifetime/ownership semantics. Having everything sharing everything is sort of a mess and leads to lack of clarity...having clear ownership can make things simpler. And since the Parent class still manages the lifetime of each Child, there's no need to have a fancy pointer from Child back to Parent--a raw pointer will work because the Child won't outlive the Parent.

What happens here when someone wants to iterate over the const vector reference handed back by children()? What do they get from *iterator? A std::unique_ptr<Child>&? Cause unique_ptr can't be copied, so it's not going to be done by-value, right?
Dereferencing the iterator returned by the non-const children() would indeed produce a reference to unique_ptr. It should work fine.
+1 for noticing that the child will not outlive the parent and hence does not need ownership. Small nitpick: Consider using std::make_shared in Parent::create. It will require friending make_shared, since the constructor is private, but the benefits of make_shared should make up for that. Same argument (although slightly weaker, since you won't save an allocation) for std::make_unique in Parent::createChild if your compiler already supports it.

mockinterface · Accepted Answer · 2014-04-07 13:37:55Z

Given your description of the uncomfortable split of the parent/children management, you can simplify as follows,

class Child; class Parent { private: Parent() {}; friend std::shared_ptr<Child> CreateChild(const std::shared_ptr<Parent>& parent); std::vector<std::shared_ptr<Child>> _children; }; class Child { private: Child(const std::shared_ptr<Parent>& parent) : _parent(parent) {}; friend std::shared_ptr<Child> CreateChild(const std::shared_ptr<Parent>& parent); std::weak_ptr<Parent> _parent; }; std::shared_ptr<Child> CreateChild(const std::shared_ptr<Parent>& parent) { auto child = std::shared_ptr<Child>(new Child(parent)); parent->_children.push_back(child); return child; }

Also notice that there is no shared_from_this here, maybe it is necessary for your other purposes, but it isn't necessary to manage the parent-child relationship.

Collectives™ on Stack Overflow

How should I construct an instance intended to be managed with std::shared_ptr?

2 Answers 2

3 Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

Comments

Related