CODEWITHSUNDEEP
c++boostsmart-pointers
Drism
Drism

Reputation: 309

Conditional boost::shared_ptr initialization?

In an effort to write better code, I've committed to using boost's smart pointers.

Should a boost::shared_ptr always be initialized like so?

boost::shared_ptr<TypeX> px(new TypeX);

My confusion has risen from a code block similar to this:

void MyClass::FillVector()
{
    boost::shared_ptr<TypeX> tempX;
    if(//Condition a)
    {
        boost::shared_ptr<TypeX> intermediateX(new TypeA);
        tempX = intermediateX;
    }
    else
    {
        boost::shared_ptr<TypeX> intermediateX(new TypeB);
        tempX = intermediateX;
    }
    _typeXVec.push_back(tempX); // member std::vector< boost::shared_ptr<TypeX> >
}

Is there an accepted way to skip that intermediate shared_ptr while keeping it in scope to push back to _typeXVec?

Thank you.

Edit: Wanted to clarify that TypeA and TypeB are both children of TypeX.

Upvotes: 3

Views: 1606

Answers (2)

bytemaster
bytemaster

Reputation: 602

boost::shared_ptr<X> p;
if( condition ) {
  p.reset( new A() );
}else {
  p.reset( new B() );
}

Upvotes: 6

James McNellis
James McNellis

Reputation: 355039

Should a boost::shared_ptr always be initialized like so?

Yes, per the Boost shared_ptr Best Practices.

This is not the only safe way to have a smart pointer take ownership of a dynamically allocated object, but it is a common pattern that can be used practically everywhere. It's easy for anyone who is familiar with this pattern to look at the code and know that it is correct.

(So, for example, I'm pretty sure that tempX.reset(new TypeA); would also be safe, but I'm not 100% sure. I'd have to check the documentation, and I'd probably want to check the implementation. Then I'd have to think through all of the possible failure cases and see that all of them are handled correctly. If you follow the pattern, you and the people who maintain the code later don't have to waste time thinking through these issues.)

One note, however: rather than the assignment, however, I would recommend either of the following:

using std::swap;
swap(tempX, intermediateX);

tempX = std::move(intermediateX);

These remove the need for the unnecessary atomic reference count update required by assignment.

Upvotes: 3

Related Questions