CODEWITHSUNDEEP
c++11shared-ptr
ipmcc
ipmcc

Reputation: 29886

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

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?

Upvotes: 2

Views: 123

Answers (2)

mockinterface
mockinterface

Reputation: 14860

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.

Upvotes: 1

John Zwinck
John Zwinck

Reputation: 249123

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.

Upvotes: 2

Related Questions