Alternative to using shared_from_this() in a constructor of a base class

Question

This question is somewhat related to this one, but not the same... please don't refer me to it...

In another question I asked (a different question) about a class similar to this one:

#include <memory>
#include <set>

using namespace std;

class Base : public enable_shared_from_this<Base>
{
public:
    explicit Base(Base *parent = nullptr) : parent(parent) { parent->children.insert(shared_from_this()); }

private:
    Base *parent; // weak pointer
    set<shared_ptr<Base>> children;
};

Some of the responds mentioned (correctly) that I can't use shared_from_this() in the constructor.

I read about this issue that the best solution (short of using boost::intrusive_ptr) would be to use a static factory method instead of a constructor but my base class is actually virtual so my code turns out this way:

#include <memory>
#include <set>

using namespace std;

class Base : public enable_shared_from_this<Base>
{
protected:
    Base() {};
    void SetParent(Base *_parent)
    {
        parent = _parent;
        parent->children.insert(shared_from_this());
    }

private:
    Base *parent = nullptr;
    set<shared_ptr<Base>> children;
};

class Derived : public Base
{
public:
    static Derived *Create(Base *parent = nullptr)
    {
        Derived *obj = new Derived();
        if (parent) obj->SetParent(parent);
        return obj;
    }

private:
    Derived() : Base() {};
};

However, this would force me to write boilerplate code for every child class, therefore the natural evolution would be to write it as a template factory at the base class, but since I want the derived class constructor to be private (to force using the static factory) I have to declare the base class as a friend in the derived class, resulting that the following code:

#include <memory>
#include <set>

using namespace std;

class Base : public enable_shared_from_this<Base>
{
public:
    template<typename T> static T* Create(Base *parent = nullptr)
    {
        T *obj = new T();
        if (parent) obj->SetParent(parent);
        return obj;
    }

protected:
    Base() {};
    void SetParent(Base *_parent)
    {
        parent = _parent;
        parent->children.insert(shared_from_this());
    }

private:
    Base *parent = nullptr;
    set<shared_ptr<Base>> children;
};

class Derived : public Base
{
private:
    friend class Base;
    Derived() : Base() {};
};

So my question is... is there a more elegant way to do this (preferably shorter and without boost and friend classes)...?

Answer 1

Here is my take on it - and it is controversial. Be warned. It is a simpler version of intrusives but it has its value. Take it for what's worth it.

First you define a base class with a counter inside. If your code is not expected to be multithreaded, you dont need the atomic for speed.

#include <memory>
#include <atomic>
#include <vector>
#include <iostream>
struct ReferenceCounter {
    std::atomic<int> counter;
};

Then create a base class based on that reference counted class. It has a vector (not a set) for children. (If you add a child twice, shouldnt it be double ref counted?). The purpose of the virtual inheritance is to avoid the dreaded diamond problem.

class Base : public virtual ReferenceCounter
{
public:
...
private:
    Base *parent = nullptr;
    std::vector<Base*> children;
};

The default constructor will take a Base pointer. I simplified away your Create static method - sorry, you can put it back it's not a problem.

    Base( Base* parent ) {
        std::cout << "Creating object " << this << " with parent " << parent << std::endl;
        if (parent!=nullptr) { 
            SetParent(parent);
        }
    }

The destructor will traverse all the children and decrement all their reference counters. If any gets to zero, delete it.

    virtual ~Base() {
        std::cout << "Erasing object " << this << std::endl;
        for ( Base* ptr : children ) {
            ptr->counter--;
            if ( ptr->counter == 0 ) {
                delete ptr;
            }
        }        
        if ( parent!=nullptr ) {
            parent->RemoveChild( this );
        }
    }

Then comes the machinery to add children properly

protected:
    void addChild( Base* ptr ) {
        if ( ptr != nullptr ) {
            ptr->counter++;
            children.push_back( ptr );
        }
    }
    void SetParent(Base *_parent)
    {
        parent = _parent;
        parent->addChild( this );
    }
    void RemoveChild( Base* ptr ) {
        for ( size_t j=0; j<children.size(); ++j ) {
            if ( children[j]==ptr ) {
                children.erase( std::remove_if( children.begin(),children.end(), 
                                [ptr]( Base* child ) { return child==ptr; }));
            }
        }
    }

Then there is minimal boilerplate in the derived classes.

class Derived : public Base
{
    public:
    Derived( Base* parent = nullptr ) : Base(parent) {}
};

Beware this mechanism will leak if there is cyclic references.

You can run some tests to see if it works:

void test1() {
    std::cout << ">>>>>>>>>>>>>>> Test 1" << std::endl;
    Derived* parent = new Derived();
    Derived* d1 = new Derived( parent );
    Derived* d2 = new Derived( d1 );
    Derived* d3 = new Derived( d2 );
    std::cout << "Deleting parent" << std::endl;
    delete parent;
}

Produces

>>>>>>>>>>>>>>> Test 1
Creating object 0xaadec0 with parent 0
Creating object 0xaadf00 with parent 0xaadec0
Creating object 0xaadf60 with parent 0xaadf00
Creating object 0xaadfc0 with parent 0xaadf60
Deleting parent
Erasing object 0xaadec0
Erasing object 0xaadf00
Erasing object 0xaadf60
Erasing object 0xaadfc0

And another test, now with a hierarchical topology where one child is deleted

void test2() {
    std::cout << ">>>>>>>>>>>>>>> Test 2" << std::endl;
    Derived* parent = new Derived();
    Derived* d1 = new Derived( parent );
    Derived* d2 = new Derived( d1 );
    Derived* d3 = new Derived( d2 );
    std::cout << "Deleting d2" << std::endl;
    delete d2;
    std::cout << "Deleting parent" << std::endl;
    delete parent;
}

Produces

>>>>>>>>>>>>>>> Test 2
Creating object 0xaadec0 with parent 0
Creating object 0xaadf00 with parent 0xaadec0
Creating object 0xaadf60 with parent 0xaadf00
Creating object 0xaadfc0 with parent 0xaadf60
Deleting d2
Erasing object 0xaadf60
Erasing object 0xaadfc0
Deleting parent
Erasing object 0xaadec0
Erasing object 0xaadf00

I put the entire code on Godbolt