CODEWITHSUNDEEP
c++11design-patternssmart-pointers
PiotrK
PiotrK

Reputation: 4453

Two step constructions for enable_shared_from_this object that needs to pass std::shared_ptr<self> to children created in constructor

I know that additional initialization methods are evil, as they leave a very nasty option for having object half-constructed and as result all methods needs to check for this. But what about this situation?

class config;
class cfg_item final
{
    private:
        friend class config;
        cfg_item(std::weak_ptr<config> owner) : owner(owner) { }
        std::weak_ptr<config> owner;
}
class config final : private std::enable_shared_from_this<config>
{
    public:
        config()
        {
             items.emplace(std::make_shared<cfg_item>(weak_from_this())); // Will crash!
        }
    private:
        std::vector<std::shared_ptr<cfg_item>> items;
}
int main(int argc, char * argv[])
{
    std::shared_ptr<config> cfg = std::make_shared<config>();
}

I KNOW WHY IT CRASHES. The std::shared_ptr in the main is not yet initialized with shared pointer to config instance, so constructor does not know how to make weak_from_this and just raises std::bad_weak_ptr exception because there are no valid std::shared_ptr pointing to this at constructor's call time.

The question is: how can I avoided the whole thing? I believe the only way I see would be to add separate initialization method, which is evil as I've already mentioned...

As note about real code: the constructors loads cfg_item from external source. It is assumed that all cfg_items are available for the entire lifetime of config. The weak pointers back to config are mandatory, as cfg_item must push all changes done to it back to config to save to external source

Upvotes: 2

Views: 323

Answers (1)

Ami Tavory
Ami Tavory

Reputation: 76297

If you look at the answers to this question, there are strong arguments why an external initialization function is necessary. However, you rightfully write

I know that additional initialization methods are evil, as they leave a very nasty option for having object half-constructed and as result all methods needs to check for this.

it's possible to reduce this problem. Say you have a class foo, with the protocol that each time a foo object is constructed, foo::init() needs to be called. Obviously, this is a brittle class (client code will eventually omit calls to init()).

So, one way is to make the (non-copy / non-move) constructors of foo private, and create a variadic static factory method that creates objects, then calls init():

#include <utility>

class foo { 
private:
    foo() {}
    foo(int) {}
    void init() {}

public:
    template<typename ...Args>
    static foo create(Args &&...args) {
        foo f{std::forward<Args>(args)...};
        f.init();
        return f;
    }
};

In the following code

    template<typename ...Args>
    static foo create(Args &&...args) {
        foo f{std::forward<Args>(args)...};
        f.init();
        return f;
    }

note that this single method can be used for all constructors, regardless of their signature. Furthermore, since it is static, it is external to the constructor, and doesn't have the problems in your question.

You can use it as follows:

int main() {
    auto f0 = foo::create();
    auto f1 = foo::create(2);
    // Next line doesn't compile if uncommented
    // foo f2; 
}

Note that it's impossible to create an object without this method, and the interface doesn't even contain init.

Upvotes: 2

Related Questions