19

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?

Kyle
  • 4,487
  • 3
  • 29
  • 45
  • 18
    Google is wrong. There's a giant thread on this and other short-comings of their style guide on comp.lang.c++.moderated. The basic reason for them coming up with this is because they also (wrongly) ban exceptions. –  Mar 24 '10 at 19:01
  • 3
    @Neil: The impression I got was that Google was being heavy-handed with the style guide to accomodate people who don't know C++ well, trying to limit the damage they could do, or perhaps to enforce uniformity with legacy code. Google may have good reasons for their style guide, but it's not a good model for anybody else to copy. – David Thornley Mar 24 '10 at 19:10
  • 9
    @David That's correct. But people can get confused using floating point numbers (look at all the questions here on the topic) or even integers, but we don't ban their use. This is one of the reasons I am no fan of style guides. –  Mar 24 '10 at 19:18
  • Agreed. Better to teach programmers to become better programmers than to take their tools away. – John Dibling Mar 24 '10 at 19:26
  • 3
    @Neil: I have no objection to style guides. Every shop should have one. What I dislike is style guides that may be perfectly usable in-house being used as models and references. Google has accomplished great things, but I'd be willing to bet it's not simply because of their style guide. (Yeah, I'm nitpicking, I think we're in agreement here.) – David Thornley Mar 24 '10 at 19:27
  • I agree with Neil here. Google is in a particular position, and their requirements may well not suit everyone's. Exceptions in particular, and I know what I am talking about: we have banned exceptions in the tight loops in our system because they made a measurable dent to performance (we did meassure), but we are still using them in all other parts, even in the same application. Having a rule either way would either impact performance in one end or maintainability of the non-critical parts of the application in the other. – David Rodríguez - dribeas Mar 24 '10 at 21:24
  • 2
    If I remember correctly, Google's style sheet is carefully explained: they had a huge existing `C` code base and wanted the `C++` code to interact seamlessly with it, which required that no exceptions propagated to `C` parts of the code... and rather than failing to catch them they simply banned them altogether. This of course led to a number of other decisions like the `init` method and the `error` return value. – Matthieu M. Mar 25 '10 at 07:32
  • `"'this' should not be used in a constructor"` -- Interesting point! Even if you could create shared_ptr(this) during construction, you would want to be careful handing it out before your "class invariant" is established. – Brent Bradburn Feb 24 '11 at 05:54

6 Answers6

16

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
  • 99,718
  • 31
  • 186
  • 324
  • 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 Mar 24 '10 at 18:59
  • 2
    The term is accurate and I know of no better, but I'd return a smart pointer from Create. –  Mar 24 '10 at 19:04
  • 1
    The downside of course is that you can't put the object on the stack. – Mark Ransom Mar 24 '10 at 19:24
  • 6
    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. –  Mar 24 '10 at 20:03
  • 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 Mar 24 '10 at 20:04
  • 1
    @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 Mar 24 '10 at 20:12
  • @John Just re-read my comment & realise that it might seem to be an attempt to stop people upvoting you - not! –  Mar 24 '10 at 20:15
  • @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 Mar 24 '10 at 20:23
  • @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. Mar 25 '10 at 07:35
9

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
  • 219,715
  • 46
  • 258
  • 445
  • 3
    +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. –  Mar 24 '10 at 19:06
5

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
  • 16,577
  • 3
  • 41
  • 45
  • 4
    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 Mar 24 '10 at 19:29
3

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
  • 204,818
  • 23
  • 294
  • 489
  • 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 Mar 24 '10 at 20:33
  • 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 Mar 24 '10 at 21:38
  • What I like about the `shared_ptr` is the code insulation property they have: you can manipulate `shared_ptr` 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. Mar 25 '10 at 07:38
0

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
  • 37,849
  • 12
  • 53
  • 71
0

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
  • 11,888
  • 3
  • 47
  • 79