Recommendations for deleting a resource in an inheritance hierarchy

Question

Consider the following code:

class Base { protected: int* ptr_; public: virtual ~Base() { /* delete ptr_ ?? */ } } class A : public Base { public: A() { ptr_ = new int; } ~A() { /* delete ptr_ ?? */ } } 

I have a little argument with my colleague.
Which destructor should remove ptr_ ?
I think it should be in A , because it is where it is allocated.
He thinks it should be in Base , because where the pointer is.

Please give possible pitfalls why one method is better than another (if there is one, and this is not just a matter of taste)

EDIT
Many people wonder about design. In practice, the code is much more complex and models an image processing system that includes several cameras. The role of the database is to manage system resources (there are several configurations). Class A and other derived types like it define the Base configuration and initialize the system with a specific configuration. I think now you can ask if Inheritance is the right choice over a composition. A is-a Base , only a certain kind, and that is why we chose inheritance. In this case, ptr_ is a pointer to a specific camera driver, which will be determined by the derived type, so it stands out there.

Answer 1

You are worried about code duplication, but I'm afraid it clouded your mind.

I know that DRY receives a lot of press (and for good reason), but you misunderstood it. There is a difference between non-duplicate code and not duplicate functionality.

DRY does not duplicate functionality. There may be code templates that look the same, but are used for different purposes, which should not merge, since each goal can move independently (and not merging is a nightmare). Their disassembly is random.

Here. You arbitrarily force int* ptr_ in the Base class (which doesn't use it) because, of course, all derived classes will need it. How did you know that? At the moment, it turns out that all derived classes are really needed, but this is a coincidence. They could rely on something else.

Therefore, a reasonable design is to provide an interface in the base class and leave it for each derived class to choose its own implementation:

 class Base { public: virtual ~Base() {} }; class D1: public Base { public: D1(Foo const& f): ptr_(new Foo(f)) {} private: std::unique_ptr<Foo> ptr_; }; class D2: public Base { public: D2(Bar const& f): ptr_(new Bar(f)) {} private: std::unique_ptr<Bar> ptr_; }; 

On the other hand, if the Base class was supposed to provide some general functionality that required data, then, of course, they could be implemented in the implementation of the Base class ... although it can be argued that this is a premature optimization, since again some derivatives classes can do something differently.

Answer 2

Each class must manage its own resources. When you have multiple derived classes, everyone must remember to free a pointer. This is bad because it duplicates the code.

If some derived class is allowed not to assign a new value to the pointer, then set it to zero in the constructor of the base class. If some derived class should be allowed to assign an automatic variable address to the pointer, then review your design.

Use smart pointers so that the problem disappears completely (when I grep delete * in my code base, I did not find a match :)

Answer 3

The question is to develop more than one implementation option. I am inclined to agree with your colleague, if the pointer is in the database, it seems to indicate that the underlying object is responsible for managing the resource, and not for the derived type.

In fact, it is strange that the derived constructor modifies the base element, it probably makes sense to pass a pointer to the base constructor, in which case the semantics will be clearer: the base receives the resource during construction and is responsible for managing it during its life .

If the database does not have to manage the resource, then why is the pointer at this level? Why is he not a member of a derived type?

Note that when managing a resource, you should take into account the rule for three ¹ : If you need to implement one of the copy constructors, an assignment operator or a destructor, then you probably want to implement three of them (think additionally about implementing no-throw swap , which will be useful).

As soon as you add the copier linker and the target operator to the project, you will probably see that the natural place to manage the resource is where it is stored.

¹ This rule is general, because there are good reasons for not always following them all. For example, if you manage your resources within members that implement RAII (as smart pointers), you may need to provide copy-construction and assign-operator, as for deep copy semantics, but destruction will not be required. Alternatively, in some contexts you can disable * copy-construction * and / or assignments.

Answer 4

In fact, ptr_ should be private ! (See Scott Meyers Effective C ++ or any object orientation tutorial.)

Now the question does not even represent itself: only the base class can manage its resource. The resulting class can only initialize it by passing a request to its base (preferably through the constructor):

 A() : Base(new int()) { } 

Answer 5

I would prefer to remove it in the base class, because If I release it in the destructor of the Derived class (which is called before the destructor of the Base class), there is a chance that it can be used accidentally in the base class.

In addition, the pointer expires in the Base class, so it should be a place to free it. In fact, I would highlight the pointer inside the Base class, not the Derived class.

It's best to use RAII rather than dividing the pointer explicitly; let Smart Pointers take care of freeing themselves.

Answer 6

ptr_ should not be in the base class, because nothing in the base class uses it. It must be in a derived class.

If you have it, I agree with you, it should be removed in the derived desteuctor classes. The base class does not even know if this pointer was used, why should it delete it?

Answer 7

The best practice is an object that creates a resource, which should remove the resource - this is their responsibility.

Answer 8

For me, it's best to have the same object that is responsible for distribution and non-distribution. That way, in your code, class A will be responsible for delete , since it executed new .

But: why did A highlight ptr_ instead of Base ? This is a real question. There's a design problem for me as ptr_ belongs to Base . Either it should belong to A , or Base should be responsible for memory management.

Answer 9

I would do it using RAII .

 class RaiiResource { private: int* ptr_; public: RaiiResource() { ptr_ = new int; } ~RaiiResource() { delete ptr_; } int* getResource() { return ptr_; } } class Base { protected: RaiiResource m_raiiresource; void anyMethod() { int* pIntNeeded = m_raiiresource.getResource();/*when you need the resource*/ } public: virtual ~Base() {} } class A : public Base { public: A() {} ~A() { } void anyOtherMethod() { int* pIntNeededAgain = m_raiiresource.getResource(); /*when you need the resource again somewhereelse*/ } } 

Another way would be to use std :: shared_ptr, which allows you to control the lifespan of objects between different objects, which is the case in your case (Base and A share the need for an object to exist (int))