views:

157

answers:

1

Hi,

I am not sure about a good way to initialize a shared_ptr that is a member of a class. Can you tell me, whether the way that I choose in C::foo() is fine, or is there a better solution?

class A
{
  public:
    A();
};

class B
{
  public:
    B(A* pa);
};

class C
{
    boost::shared_ptr<A> mA;
    boost::shared_ptr<B> mB;
    void foo();
};

void C::foo() 
{
    A* pa = new A;
    mA = boost::shared_ptr<A>(pa);
    B* pB = new B(pa);
    mB = boost::shared_ptr<B>(pb);
}
+7  A: 

Your code is quite correct (it works), but you can use the initialization list, like this:

C::C() :
  mA(new A),
  mB(new B(mA.get())
{
}

Which is even more correct and as safe.

If, for whatever reason, new A or new B throws, you'll have no leak.

If new A throws, then no memory is allocated, and the exception aborts your constructor as well. Nothing was constructed.

If new B throws, and the exception will still abort your constructor: mA will be destructed properly.

Of course, since an instance of B requires a pointer to an instance of A, the declaration order of the members matters.

The member declaration order is correct in your example, but if it was reversed, then your compiler would probably complain about mB beeing initialized before mA and the instantiation of mB would likely fail (since mA would not be constructed yet, thus calling mA.get() invokes undefined behavior).


I would also suggest that you use a shared_ptr<A> instead of a A* as a parameter for your B constructor (if it makes senses and if you can accept the little overhead). It would probably be safer.

Perhaps it is guaranteed that an instance of B cannot live without an instance of A and then my advice doesn't apply, but we're lacking of context here to give a definitive advice regarding this.

ereOn
Given that you're relying on it, it's worth talking about the how the order of construction of members works and the sequence points involved.
Charles Bailey
@ereOn - A return type for a constructor ?
DumbCoder
@DumbCoder: Copied from the question where it's also wrong. I fixed it.
sbi
@Charles Bailey: The order of construction actually doesn't matter. If B were constructed first, or even if it were random, it would still be exception-safe. That is a result of non-overlapping construction, not order. Non-overlap implies that at each point where new memory is allocated, previously allocated memory is managed by a fully-constructed subobject.
MSalters
@MSalters: If the initializer for `mB` includes the call `mA.get()` (which it does) then the order of initialization very much does matter.
Charles Bailey
@MSalters: Just to be clear on what I originally meant: while the answer is correct as written, it hasn't been made completely clear why it's correct. If the order of declaration of `mA` and `mB` were reversed in the class definition then it may not be clear why `C() : mA(new A), mB(new B(mA.get())) {}` is incorrect.
Charles Bailey
@sbi: I indeed copied it from the question. Thanks.
ereOn
Fair point, that part I had considered trivial.
MSalters
@Charles Bailey: Edited my answer so it is more precise regarding the initialization order. (At least I hope so). Feel free to correct it if something seems wrong. Thanks for the suggestion.
ereOn
I'm with Charles on this. I usually try to prevent for a ctor to rely on the order of members to be initialized. If I can't avoid it, I put a __big and scary comment next to those members' definitions__.
sbi