Day_Dreamer
Day_Dreamer

Reputation: 3373

When do we have to define a destructor in derived class c++

I'm kind of new to c++ and I'm facing memory leaks which made me wonder maybe I didn't understand something right.

I'm having:

.h file

class DeliveryVehicle{
public:
    //c'tor
    DeliveryVehicle(const char* ID, Quality quality);

    //d'tor
    virtual ~DeliveryVehicle();

    ...
protected:
    VehicleParcelQueue parcelQueue_;
}

.c file

DeliveryVehicle::~DeliveryVehicle(){
    while(!parcelQueue_.empty()) parcelQueue_.pop(); 
    // pop() calls removed element destructor
}

I have a derived class, in which I count on the default destructor, and not implementing a destructor explicitly.

I would like to know, is it ok in case I don't allocate memory in the derived class using "new"?

Additionally, I implemented fixed size queue of my own which inherits the Base Class Queue from STL:

.h file

class VehicleParcelQueue:public std::queue<Parcel*>{
public:
    const static int MaxParcelsNum = 5;
    Result push(Parcel*);
};

.cpp file

typedef enum result{SUCCESS = 1,FAILURE = 0} Result;

Result VehicleParcelQueue::push(Parcel* pParcel){
if (size() >= 5) return FAILURE;
else{
    queue<Parcel*>::push(pParcel);
    return SUCCESS;
}

As can be seen, also In this case, I didn't implement the destructor explicitly. Am I prone to memory leaks?

Maybe pop doesn't calling delete for Parcle but destructor to pointers? thanks

Upvotes: 1

Views: 469

Answers (3)

opetroch
opetroch

Reputation: 4105

When an instance of VehicleParcelQueue goes out of scope, its destructor will be called, and subsequently the destructor of the base class std::queue . The destructor of std::queue will "free" the nodes of the queue, but will not call delete for the pointer stored in each node. You have to do this manually in the destructor of VehicleParcelQueue to avoid leaks. Of course, you may also not want to delete these objects if some other object still references them.

Maybe pop doesn't calling delete for Parcle but destructor to pointers?

Correct. It removes the node from the queue, but doesn't call delete on the pointer. You have to get the content of the node i.e. the pointer and call delete on it.

Upvotes: 1

quantdev
quantdev

Reputation: 23813

in addition to R Sahu answer, note that std::queue<> destructor is not virtual, any deletion from a pointer to Base will invoke undefined behavior (for that reason, you should generally not inherit from standard containers).

You should probably review your design, such as using a class member instead of inheritance, and preferring values to pointers if possible :

VehicleParcelQueue
{
    ...

    std::queue<Parcel> queue;
};

Upvotes: 4

R Sahu
R Sahu

Reputation: 206717

The default destructor in VehicleParcelQueue does not delete the objects that the pointers in the queue point to. Those are the objects that are the cause of your memory leak.

You'll have to implement a destructor for VehicleParcelQueue and make sure that you delete all the Parcel objects that are in the queue.

The previous suggestion won't work since the destructor of queue will be invoked first.

Your best bet is to use

VehicleParcelQueue:public std::queue<std::unique_ptr<Parcel>>{

or

VehicleParcelQueue:public std::queue<std::shared_ptr<Parcel>>{

Upvotes: 0

Related Questions