C++ boost::shared_ptr as a member of a class and repeatedly setting its value

Question

I am trying to figure out the cleanest way to implement using a boost::shared_ptr as a member of a class. Let's say I have class A that contains a boost::shared_ptr to a class B member. The B class could be either the base type (B) or a derived type (let's say C or D). This member would be obtained/set continuously throughout the lifetime of class A (using accessors/modifiers). B has to be polymorphic so I can't just store it as B but rather as a reference/pointer to B. I also wanted to avoid having to manage the memory on my own as well. Let's say it all looked like:

class A
{
public:
    const B& GetB() const { const B* b = m_pB.get(); return *b; } // Is the correct way to do this?
    void SetB(const B& b) { B* b = m_pB.get(); *b = b; } // Is the correct way to do this?

private:
    boost::shared_ptr<B> m_pB;
};

I know I'm probably doing something wrong but I wanted to provide an example of what I'm trying to accomplish. Thanks!

Why do you think you need to store a dynamically allocated object? What's wrong with simply storing a `B`? — sbi, Aug 10 '12 at 14:51
Why are you using a `shared_ptr` rather than holding the `B` object by value? [This is irrelevant to the question] — David Rodríguez - dribeas, Aug 10 '12 at 14:52

score 1 · Accepted Answer · answered Aug 10 '12 at 14:48

1

I think this program's design needs rethinking.

The purpose of a shared pointer is that you leave the dynamically allocated resource within it so that it manages the life-cycle of that resource. If you're changing what's in there, then you're responsible for managing the life of that resource after you replace the pointer in there and that's going to lead to confusion.

You should probably just assign over the m_pB variable with a new shared pointer so the original one goes out of scope and frees the resource that was being held in the first place. So, assign over the shared pointer, not over the pointer it holds.

answered Aug 10 '12 at 14:48

John Humphreys

37,047
37
155
255

So the setter would look like: void SetB(const B& b) { m_pB = boost::shared_ptr(new B(b)); } and getter would look like: const B& GetB() const { const B* b = m_pB; return *b; }? – user1040229 Aug 10 '12 at 14:51
That would be an improvement, then your resource would be deleted/freed and you wouldn't have to do it manually :) Or you can have the user create the shared pointer in the main scope and pass it in directly so you don't need the original b in the first place - saves you a copy. – John Humphreys Aug 10 '12 at 14:56

score 0 · Answer 2 · answered Aug 10 '12 at 14:52

In your setter, you are assigning the new data to the existing element, not updating the pointer.

The correct way would be:

void SetB(shared_ptr<B> ptr)
{
     m_pB = ptr;
}

So doing this you will be transferring the ownertship of the object to the class. For using, you would write code like:

A a;

shared_ptr<B> ptrB(new C()); //or new D, whatever
a.SetB(ptrB);

Another option, if the code thats creates B does not use shared_ptr at all and the shared_ptr will be encapsulated inside your class, is to change the setter to get a pointer to B, like:

void SetB(B *pB)
{
     m_pB.reset(pB);
}

In the end, if this last sample is what you need and the shared_ptr will be restricted only to the class A, I would sugest using scoped_ptr instead of shared_ptr.

C++ boost::shared_ptr as a member of a class and repeatedly setting its value

2 Answers2