Question

boost::shared_ptr really bothers me. Certainly, I understand the utility of such a thing, but I wish that I could use the shared_ptr<A> as an A*. Consider the following code

class A
{
public:
    A() {}
    A(int x) {mX = x;}
    virtual void setX(int x) {mX = x;}
    virtual int getX() const {return mX;}
private:
    int mX;
};


class HelpfulContainer
{
public:
    //Don't worry, I'll manager the memory from here.
    void eventHorizon(A*& a)
    {
        cout << "It's too late to save it now!" << endl;
        delete a;
        a = NULL;
    }
};


int main()
{
    HelpfulContainer helpfulContainer;

    A* a1 = new A(1);
    A* a2 = new A(*a1);
    cout << "*a1 = " << *a1 << endl;
    cout << "*a2 = " << *a2 << endl;
    a2->setX(2);
    cout << "*a1 = " << *a1 << endl;
    cout << "*a2 = " << *a2 << endl;
    cout << "Demonstrated here a2 is not connected to a1." << endl;

    //hey, I wonder what this event horizon function is.
    helpfulContainer.eventHorizon(a1);

    cout << "*a1 = " << *a1 << endl;//Bad things happen when running this line.
}

Whoever created the HelpfulContainer wasn't thinking about others wanting to retain pointers to A objects. We can't give HelpfulClass boost::shared_ptr objects. But one thing we could do is use the pimlp idiom to create a SharedA which itself is an A:

class SharedA : public A
{
public:
    SharedA(A* a) : mImpl(a){}
    virtual void setX(int x) {mImpl->setX(x);}
    virtual int getX() const {return mImpl->getX();}
private:
    boost::shared_ptr<A> mImpl;
};

And then the main function can look something like this:

int main()
{
    HelpfulContainer helpfulContainer;

    A* sa1 = new SharedA(new A(1));
    A* sa2 = new SharedA(sa1);
    cout << "*sa1 = " << *sa1 << endl;
    cout << "*sa2 = " << *sa2 << endl;
    sa2->setX(2);
    cout << "*sa1 = " << *sa1 << endl;
    cout << "*sa2 = " << *sa2 << endl;
    cout << "this demonstrates that sa2 is a shared version of sa1" << endl;

    helpfulContainer.eventHorizon(sa1);
    sa2->setX(3);
    //cout << "*sa1 = " << *sa1 << endl;//Bad things would happen here
    cout << "*sa2 = " << *sa2 << endl; 
    //but this line indicates that the originally created A is still safe and intact.
    //only when we call sa2 goes out of scope will the A be deleted.
}

So, my question is this: Is the above pattern a good pattern, or is there something I'm not considering yet. My current project inherited a HelpfulContainer class like above that's deleting the pointers that I need, but I still need the data structure present in the HelpfulContainer.


Update: This question is a follow-on question.

Was it helpful?

Solution

The whole point of shared_ptr is that it (and its copies) own the object that it points to. If you want to give an A to a container that manages its lifetime then you shouldn't be using a shared_ptr at all as it doesn't meet your needs; HelpfulContainer only knows how to be the sole owner of a dynamically created object so you need to give it a pointer to an object that isn't owned by anything else.

I think that it is usually poor design for an object to care about its own lifetime (there are exceptions). It is usually more useful if an object can do a job and something else manages its creation and descruction, choosing the simplest lifetime strategy possible (e.g. local/automatic variable).

If you absolutely have to share ownership between two things that don't co-operate (such as shared_ptr and HelpfulContainer) then you will have to use some sort of proxy technique.

In this case, though, it just looks like HelpfulContainer just isn't that helpful for your situation.

OTHER TIPS

I'm not sure what this does for you.

If helpfulContainer.eventHorizon() always deletes its parameter, then why not just pass a new copy of (the original) A class:

  helpfulContainer.eventHorizon(new A(sa1));

Or, if helpfulContainer.eventHorizon() only sometimes deletes its parameter, then making the call as

  helpfulContainer.eventHorizon(new SharedA(sa1)); 

will leak both the SharedA and the original A (sa1) on those occasions when it chooses not to delete.

So you are creating a Stand-In (SharedA) for which deletion is okay. Even though this is kinda awkward, I guess it's necessary to work with your legacy API. To slightly improve this: Allow construction of SharedA from a shared_ptr, but not the other way around - and then only use the SharedP when you absolutely must:

int main()
{
  HelpfulContainer helpfulContainer;

  boost::shared_ptr<A> sa1(new A(1));

  // deletes its parameter, but that's okay
  helpfulContainer.eventHorizon(new SharedA(sa1)); 
}

Implicit conversions to the underlying pointer type are inconsistent with the intended use of shared_ptr in that you can extremely easily pass the shared_ptr to a function etc without realizing it.

It sounds to me like HelpfulContainer is anything BUT helpful and should be fixed or ditched.

If that's not possible then probably the best way is to just copy the A you want to pass in and pass the copy to the container.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top