CODEWITHSUNDEEP
c++pointersmemory-management
Renan
Renan

Reputation: 1990

Pointer class member and deleting responsibilities

If my class has a pointer of some sort that can be set by the class clients, how should I deal with deletion?

Example:

class A {
};

class B {
public:
   void setA(A* a) {
       this->a = a;
   }
private:
   A* a;
};

How should be the destructor of the class B? Should it delete a? As I see, there are two way a user can set this pointer:

... // Assume B object b being created elsewhere
A aObj;
A* aPtr = new A();

b.setA(&aObj); // It is not OK to use delete, and class member will
               // point to invalid memory location once aObj goes out
               // of scope
b.setA(aPtr);  // Using new will make the pointer available even after
               // this block of code
...

So what is the right way of deleting b? Should I always perform a new in my set method?

Upvotes: 0

Views: 2126

Answers (6)

MGZero
MGZero

Reputation: 5963

From the looks of this, you are trying to create a reference to your A object within your B class.

To clean up B properly, you would have to check if A is null first. Something like...

~B()
{
    if (A)
    {
        delete A;
        A = 0;
    }
}

Keep in mind this also deletes the A object outside of the class, because they're referencing the same memory. So in a case like this, you could VERY easily reference a pointer to invalid memory if you deleted the A object.

I would not use this with a local variable either btw, as you'll lose the address of it when it goes out of scope. However, a local pointer on the other hand..well, then you won't have to worry about referencing invalid memory once you've left the scope of where A was created.

Upvotes: 0

RoundPi
RoundPi

Reputation: 5947

I think there would normally be be 3 scenarios, see code below:

//class B doesn't own a
class B {
public:
   void setA(A& a) {
       m_a = a;
   }
private:
   A& m_a;              //Only a reference , so need to worry about delete
};


//class B owns A
class B {
public:
    void setA(std::auto_ptr<A>& a) {
       m_a.reset(a.release());
   }
private:
    boost::scoped_ptr<A> m_a;   //m_a got deleted when instance of B lifetime end
};


//class B shared A with someone else 
class B {
public:
    void setA(boost::shared_ptr<A>& a) {
       m_a = a;
   }
private:
    boost::shared_ptr<A> m_a;   //m_a got deleted when no one need this pointer anymore(reference counting reduced to 0)
};

Upvotes: 1

Ronald
Ronald

Reputation: 1542

During your design analysis you will have to answer the following questions:

  1. Does object A depends on the lifetime of object B, if yes then use "composition" in which case object B will create A and is responsible for deleting it before object B itself is destroyed.
  2. If object A is independent of the lifetime of object B then use "aggregation". You supply object A to B via B's constructor or a set method. Object B does not have to worry about destroying object A but you will have to be certain that during the lifetime of B that A is in a valid state.
  3. If A depends on object B's lifetime but you need to create A before B, then do "dependency injection". It's like aggregation in that you can pass A to B in the constructor or set method, but A is exclusively used by B in this case and no other object is using A. B deletes A before its own destruction.

Upvotes: 0

Mark Ransom
Mark Ransom

Reputation: 308276

You have a design decision to make. Who should own the object?

  • The B object owns the A object, and setA passes ownership to B. B's destructor should delete the A.
  • Some outer code owns the A object. B will not delete it, but will depend on that outer code to destroy it at the proper time.
  • A smart pointer tracks the references to the A object and deletes it automatically when all references are destroyed.

The third option with the smart pointer is the simplest and most reliable, but all 3 choices can be made to work. The trick is to pick one and be deliberate about it.

Upvotes: 0

Luc Danton
Luc Danton

Reputation: 35449

How should be the destructor of the class B? Should it delete a?

You, the author of the class, decides of its semantics. Don't think in terms of pointers, references, and deletes. Think in terms of design: what's the relation between A and B? What does B needs a A for?

Two common types of relation are delegation and composition. Delegation would mean that client code uses the setA member to have the instance aware of some other B instance that it may use for further uses. Composition would mean that the setA member is used to initialize an internal part of the instance.

One possible implementation of delegation is using a member pointer. I'd recommend passing a reference to setA to assign to that pointer; it sidesteps the issue of checking for 0 and makes it obvious to client code that there is no ownership issue to deal with. This is compatible with polymorphic types.

One possible implementation of composition is using a A member, and passing by reference to const or by value to assign to it. Another is to use a smart pointer, especially if A is meant to be used polymorphically. Passing by smart pointer is the simplest thing to do -- but you'll have to check for 0 and/or document the cast.

No matter what you decide to use (which doesn't have to be in this list anyway), use code as a tool to achieve your purpose or design. Don't let code dictate your thoughts.

In all my example implementations you don't have to do anything special in the destructor.

Upvotes: 7

Kerrek SB
Kerrek SB

Reputation: 477188

You should really really not have such a class in the first place. Instead, use a resource managing container like shared_ptr or unique_ptr to hold the pointer to a dynamically allocated object.

As you can easily see, there's no way you'll manage to keep track of who's responsible for what if you randomly allocate dynamic objects all over the place. What about copying and assignment of your class? What about exceptions in the constructor? Don't do it.

Upvotes: 2

Related Questions