views:

614

answers:

6

I have objects which create other child objects within their constructors, passing 'this' so the child can save a pointer back to its parent. I use boost::shared_ptr extensively in my programming as a safer alternative to std::auto_ptr or raw pointers. So the child would have code such as shared_ptr<Parent>, and boost provides the shared_from_this() method which the parent can give to the child.

My problem is that shared_from_this() cannot be used in a constructor, which isn't really a crime because 'this' should not be used in a constructor anyways unless you know what you're doing and don't mind the limitations.

Google's C++ Style Guide states that constructors should merely set member variables to their initial values. Any complex initialization should go in an explicit Init() method. This solves the 'this-in-constructor' problem as well as a few others as well.

What bothers me is that people using your code now must remember to call Init() every time they construct one of your objects. The only way I can think of to enforce this is by having an assertion that Init() has already been called at the top of every member function, but this is tedious to write and cumbersome to execute.

Are there any idioms out there that solve this problem at any step along the way?

+10  A: 

Google's C++ programming guidelines have been criticized here and elsewhere again and again. And rightly so.

I use two-phase initialization only ever if it's hidden behind a wrapping class. If manually calling initialization functions would work, we'd still be programming in C and C++ with its constructors would never have been invented.

sbi
+1. It's important to remember Google's guidelines were written primarily for *their* use, not for the entire C++ community, and take all of their existing code and practices into consideration, rather than serving as a guide for how new general projects should be written.
Roger Pate
+12  A: 

Use a factory method to 2-phase construct & initialize your class, and then make the ctor & Init() function private. Then there's no way to create your object incorrectly. Just remember to keep the destructor public and to use a smart pointer:

#include <memory>

class BigObject
{
public:
    static std::tr1::shared_ptr<BigObject> Create(int someParam)
    {
        std::tr1::shared_ptr<BigObject> ret(new BigObject(someParam));
        ret->Init();
        return ret;
    }

private:
    bool Init()
    {
        // do something to init
        return true;
    }

    BigObject(int para)
    {
    }

    BigObject() {}

};


int main()
{
    std::tr1::shared_ptr<BigObject> obj = BigObject::Create(42);
    return 0;
}

EDIT:

If you want to object to live on the stack, you can use a variant of the above pattern. As written this will create a temporary and use the copy ctor:

#include <memory>

class StackObject
{
public:
    StackObject(const StackObject& rhs)
        : n_(rhs.n_)
    {
    }

    static StackObject Create(int val)
    {
        StackObject ret(val);
        ret.Init();
        return ret;
    }
private:
    int n_;
    StackObject(int n = 0) : n_(n) {};
    bool Init() { return true; }
};

int main()
{
    StackObject sObj = StackObject::Create(42);
    return 0;
}
John Dibling
By the way, I'm not sure that the correct terminology for this device is "factory method," but it is what I have always called it.
John Dibling
The term is accurate and I know of no better, but I'd return a smart pointer from Create.
Roger Pate
The downside of course is that you can't put the object on the stack.
Mark Ransom
John Dibling
John Dibling
Although there is nothing wrong with John's answer, I'm a bit worried that the upvotes it is getting might suggest to newbies that this is how all classes should implement construction in C++. And it isn't - it should be used quite rarely.
anon
I am a fan of not using `shared_ptr` as return type in a function that will not actually share the ownership with you... `auto_ptr` would be a better choice for the factory method in my opinion (the user has freedom to `release` the pointer into any other type of smart pointer, which in the case of `shared_ptr` is impossible)
David Rodríguez - dribeas
@Neil: Agreed, this method is not needed in the vast majority of cases. But my read of the OP was that Kyle was trying to find a way to do complex initialization outside of the ctor that was intuitive to use on the client side correctly. We don't often need to use 2-phase initialization, but when we do, this is one way to do it.
John Dibling
anon
@John: Once you have a wrapper object anyway, why not give it the interface of the wrapped object? Users could then simply write `StackObject sObj;` and be done with all the factory hassle. (Anyone remember Coplien's Letter-Envelope?)
sbi
@Neil: I knew what you meant. Its all good.
John Dibling
@sbi: I agree that here a `Proxy` seems quite handy, though I suppose it would mean duplicating the interface since `C++` does not have any delegation feature...
Matthieu M.
A: 

A object that requires complex construction sounds like a job for a factory.

Define an interface or an abstract class, one that cannot be constructed, plus a free-function that, possibly with parameters, returns a pointer to the interface, but behinds the scenes takes care of the complexity.

You have to think of design in terms of what the end user of your class has to do.

quamrana
+4  A: 

Depending on the situation, this may be a case where shared pointers don't add anything. They should be used anytime lifetime management is an issue. If the child objects lifetime is guaranteed to be shorter than that of the parent, I don't see a problem with using raw pointers. For instance, if the parent creates and deletes the child objects (and no one else does), there is no question over who should delete the child objects.

KeithB
And putting the parent pointer into a smart pointer leaves the child in control of the parent's lifetime, which is just a bug waiting to happen. Delete the last child, and *poof!* you get the rug pulled out from under you.
Mark Ransom
+2  A: 

KeithB has a really good point that I would like to extend (in a sense that is not related to the question, but that will not fit in a comment):

In the specific case of the relation of an object with its subobjects the lifetimes are guaranteed: the parent object will always outlive the child object. In this case the child (member) object does not share the ownership of the parent (containing) object, and a shared_ptr should not be used. It should not be used for semantic reasons (no shared ownership at all) nor for practical reasons: you can introduce all sorts of problems: memory leaks and incorrect deletions.

To ease discussion I will use P to refer to the parent object and C to refer to the child or contained object.

If P lifetime is externally handled with a shared_ptr, then adding another shared_ptr in C to refer to P will have the effect of creating a cycle. Once you have a cycle in memory managed by reference counting you most probably have a memory leak: when the last external shared_ptr that refers to P goes out of scope, the pointer in C is still alive, so the reference count for P does not reach 0 and the object is not released, even if it is no longer accessible.

If P is handled by a different pointer then when the pointer gets deleted it will call the P destructor, that will cascade into calling the C destructor. The reference count for P in the shared_ptr that C has will reach 0 and it will trigger a double deletion.

If P has automatic storage duration, when it's destructor gets called (the object goes out of scope or the containing object destructor is called) then the shared_ptr will trigger the deletion of a block of memory that was not new-ed.

The common solution is breaking cycles with weak_ptrs, so that the child object would not keep a shared_ptr to the parent, but rather a weak_ptr. At this stage the problem is the same: to create a weak_ptr the object must already be managed by a shared_ptr, which during construction cannot happen.

Consider using either a raw pointer (handling ownership of a resource through a pointer is unsafe, but here ownership is handled externally so that is not an issue) or even a reference (which also is telling other programmers that you trust the referred object P to outlive the referring object C)

David Rodríguez - dribeas
I wouldn't like the idea of coupling my classes with a particular smart pointer either. With C++0x, half of my `shared_ptr` usage could be replaced with `unique_ptr` (I never wanted to share anything, I just wanted to put the pointer into a container).
UncleBens
I thought I was the only *selfish* around not wanting to *share* everything. From a semantic point of view I also like scoped pointers, even if they will not make it into the standard...
David Rodríguez - dribeas
What I like about the `shared_ptr` is the code insulation property they have: you can manipulate `shared_ptr<Foo>` with a simple `class Foo;` forward declaration and yet release the memory correctly. None of the other pointers have it (yes, it does require some work).
Matthieu M.
A: 

Do you really need to use the shared_ptr in this case? Can the child just have a pointer? After all, it's the child object, so it's owned by the parent, so couldn't it just have a normal pointer to it's parent?

Michael Kohne
I suppose it would depend if the `Child` could be handed down to another entity (copy).
Matthieu M.