Question

What is the best approach to encapsulate objects and manage their lifetime? Example: I have a class A, that contains an object of type B and is solely responsible for it.

Solution 1, clone b object to ensure that only A is able to clean it up.

class A
{
    B *b;
public:
    A(B &b)
    {
        this->b = b.clone();
    }

    ~A()
    {
        delete b; // safe
    }
};

Solution 2, directly use the passed object, we risk a potential double free here.

class A
{
    B *b;
public:
    A(B *b)
    {
        this->b = b;
    }

    ~A()
    {
        delete b; // unsafe
    }
};

In my actual case, solution #2 would fit best. However I wonder if this is considered bad code because someone might not know about the behavior of A, even if it's documented. I can think of these scenarios:

B *myB = new B();
A *myA = new A(myB);
delete myB; // myA contains a wild pointer now

Or,

B *myB = new B();
A *firstA = new A(myB);
A *secondA = new A(myB); // bug! double assignment
delete firstA; // deletes myB, secondA contains a wild pointer now
delete secondA; // deletes myB again, double free

Can I just ignore these issues if I properly document the behavior of A? Is it enough to declare the responsibility and leave it up to the others to read the docs? How is this managed in your codebase?

Was it helpful?

Solution

You should define your object so that the ownership semantics are, as much as possible, defined by the interface. As David Thornley pointed out, std::auto_ptr is the smart pointer of choice to indicate transfer of ownership. Define your class like so:

class A
{    
    std::auto_ptr<B> b;
public:    
    A(std::auto_ptr<B> b)    
    {
        this->b = b;
    }
    // Don't need to define this for this scenario
    //~A()
    //{ 
    //   delete b; // safe
    //}
};

Since the contract of std::auto_ptr is that assignment = transfer of ownership, your constructor now states implicitly that an A object has ownership of the pointer to B it's passed. In fact, if a client tries to do something with a std::auto_ptr<B> that they used to construct an A after the construction, the operation will fail, as the pointer they hold will be invalid.

OTHER TIPS

I never delete anything myself unless I really have to. That leads to errors.

Smart pointers are your friend. std::auto_ptr<> is your friend when one object owns another and is responsible for deleting it when going out of scope. boost::shared_ptr<> (or, now, std::tr1::shared_ptr<>) is your friend when there's potentially more than one object attached to another object, and you want the object deleted when there's no more references to it.

So, either use your solution 1 with auto_ptr, or your solution 2 with shared_ptr.

If you are writing code that someone else will be using later, these issues must be addressed. In this case I would go for simple reference counting (maybe with smart pointers). Consider the following example:

When an instance of the encapsulating class is assigned an object B, it calls a method to increase object's B reference counter. When the encapsulating class is destroyed, it doesn't delete B, but instead calls a method do decrease reference count. When the counter reaches zero, object B is destroyed (or destroys itself for that matter). This way multiple instances of encapsulating class can work with a single instance of object B.

More on the subject: Reference Counting.

If your object is solely responsible for the passed object then deleting it should be safe. If it is not safe than the assertion that you are solely responsible is false. So which is it? If you're interface is documented that you WILL delete the inbound object, then it is the caller responsibility to make sure you receive an object that must be deleted by you.

If you're cloning A, and both A1 and A2 retain references to B, then B's lifetime is not being controlled entirely by A. It's being shared among the various A. Cloning B ensures a one-to-one relationship between As and Bs, which will be easy to ensure lifetime consistency.

If cloning B is not an option, then you need to discard the concept that A is responsible for B's lifetime. Either another object will need to manage the various B, or you'll need to implement a method like reference counting.

For reference, when I think of the term 'Clone', it implies a deep copy, which would clone B as well. I'd expect the two As to be completely detached from each other after a clone.

I don't clone things unnecessarily or "just to be safe".

Instead I know whose responsibility it is to delete something: either via documentation, or by smart pointers ... for example, if I have a create function which instantiates something and returns a pointer to it and doesn't delete it, so that it's unclear where and by whome that thing is ever supposed to be deleted, then instead of create's returning a naked pointer I might define create's return type as returning the pointer contained within some kind of smart pointer.

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