CODEWITHSUNDEEP
c++c++11constructorsmart-pointers
memo
memo

Reputation: 3724

Problems with shared_from_this in Constructor for Chained Calls

I understand why shared_from_this doesn't work in the constructor. I've found a number of posts on here clearly explaining why. I've also seen a workaround for it. My problem is related to this but I'm looking for an elegant workaround.

I quite like this kind of design

class Foo : public enable_shared_from_this<Foo> {
public:
  typedef shared_ptr<Foo> Ptr;

  Ptr setA(int) {/* .... */ return this->shared_from_this(); } 
  Ptr setB(int) {/* .... */ return this->shared_from_this(); } 
  Ptr setC(int) {/* .... */ return this->shared_from_this(); } 
};

so I can daisy chain like this, which I find very readable 

Foo::Ptr foo = make_shared<Foo>();
foo->setA(3)->setB(4)->setC(5);

However, my setX methods do more than just assign the value to a property, they might do slightly more complex things (like set other properties etc). So I'd like to use them in my constructor. i.e.

Foo::Foo(int a, int b, int c) {
    setA(a);
    setB(b);
    setC(c);
}

Of course this crashes. However in my case I don't actually need a shared this pointer in my constructor. It's just a side-effect of my wanting to daisy chain later.

A dirty fix could be I just make a bunch of private setX_ methods which do the actual assignment and other tasks, but don't return anything. The constructor calls those. Then I have public setX methods simply call the private ones and then return the this->shared_from_this(). That's safe, but a bit of a PITA. Is it possible to check if a shared pointer to this already exists without crashing? i.e.

class Foo : public enable_shared_from_this<Foo> {
public:
  typedef shared_ptr<Foo> Ptr;

      Ptr setA(int) { /*.....*/ return getThis(); }

protected:
    Ptr getThis() {
        return safe_to_get_this ? this->shared_from_this : Ptr();
    }

};

UPDATE

I should have made this clear in my original post, apologies for not doing so. The reason I'm using smart pointers is not so I can do daisy chaining. I usually use references for that if the situation permits.

But in this case Foo (and other classes) belong to a graph like structure, they have dependencies on each other, multiple parent-child-sibling relationships etc. So I use a mix between shared and weak pointers. But I need the methods to return shared_ptrs, so I can cast to weak or check against null if need be (They are created with factory methods, and there are find/get methods too which need checking against null). Basically it's a physics system with different types of particles and constraints between particles. I originally wrote this with my own smart pointer implementation many years ago, and now I'm upgrading it to c++11. I could make the setX methods return references, and have a single getThis() method return the shared pointer (which is what I was doing before), but then I'm worried it becomes a bit inconsistent with some methods returning smart pointers others returning references. 

Foo::Ptr foo = World::create();
foo->setA().setB().setC();

foo = World::find(...);
if(foo) foo->setA().setB();

I just prefer the consistency of always returning pointers, as opposed to sometimes pointers and sometimes references. Unless there is a strict guideline for this that can be summarized succinctly.

UPDATE2

This is the (not very elegant) workaround I'm using currently. I was just wondering if there was a way of doing this without manually keeping track of _isInited;

class Foo : public enable_shared_from_this<Foo> {
public:
  typedef shared_ptr<Foo> Ptr;

  Foo() {
     _isInited = false;
     setA();
     setB();
     setC();
     _isInited = true;
  }

  Ptr setA(int) {/* .... */ return getThis(); } 
  Ptr setB(int) {/* .... */ return getThis(); } 
  Ptr setC(int) {/* .... */ return getThis(); } 

protected:
    bool _isInited;

    Ptr() {
       return _isInited ? this->shared_from_this() : Ptr();
    }
};

Upvotes: 3

Views: 485

Answers (1)

Ami Tavory
Ami Tavory

Reputation: 76297

I'd like to add to the correct comment by @JoachimPileborg. I don't think your problem really has to do with the technicalities of an aspect of how to use enable_shared_from_this, but rather when to use (smart) pointers.

Consider the following code, showing two ways to use chained calls:

struct ptr_foo                                                                                                                              
{
    ptr_foo *set_x(int x) { return this; }
    ptr_foo *set_y(int y) { return this; }
};


struct ref_foo
{
    ref_foo &set_x(int x) { return *this; }
    ref_foo &set_y(int y) { return *this; }
};


int main()
{
    ptr_foo pf;
    pf.set_x(0)->set_y(1);

    ref_foo rf;
    rf.set_x(0).set_y(1);

    return 0;
}

The first class, ptr_foo, uses raw pointers, and the second one, ref_foo, uses references.

To begin with, the second invocation, 

rf.set_x(0).set_y(1);

seems to me much more consistent than the first one,

pf.set_x(0)->set_y(1);

for the common case of stack-based objects.

Furthermore, even if you prefer the first one, I don't think there's any reason to return a smart pointer, as

  1. no allocation is taking place

  2. the protocol is not meant for obtaining a store-able pointer to a ptr_foo by calling set_?; if someone does so, it is an abuse of the protocol, as it is just meant for chained calles.

Regarding point 2, you can't shut down protocol abuses hermetically anyway. E.g., consider

ptr_foo pf;
ptr_foo *ppf = &pf;

No matter how you design ptr_foo, the user now holds a pointer to an object of that type, which was not allocated dynamically, and which can be erroneously erased.

Upvotes: 2

Related Questions