tf.rz
tf.rz

Reputation: 1367

What should be in a proper destructor?

I know a destructor is essentially a function that deallocates memory or does a "clean up" whenever you are done with it.

My question is, what goes in a proper destructor?

Let me show you some code for a class I have:

#ifndef TRUCK_H__
#define TRUCK_H__

#include <iostream>
#include "printer.h"
#include "nameserver.h"
#include "bottlingplant.h"

using namespace std;

class BottlingPlant; // forward declaration

class Truck {

    public:
    Truck( Printer &prt, 
           NameServer &nameServer, 
           BottlingPlant &plant, 
           unsigned int numVendingMachines, 
           unsigned int maxStockPerFlavour );
    ~Truck();
    void action();

    private:
    Printer* printer;       // stores printer
    NameServer* ns;         // stores nameserver
    BottlingPlant* bottlingPlant;   // stores bottlingplant
    unsigned int numVM;     // stores number of vendingmachine
    unsigned int maxStock;      // stores maxStock
    unsigned int cargo[4];      // stores the cargo.

};

Here is the constructor:

Truck::Truck( Printer &prt, 
              NameServer &nameServer, 
              BottlingPlant &plant, 
              unsigned int numVendingMachines, 
              unsigned int maxStockPerFlavour ) {
    printer = &prt;
    printer->print( Printer::Truck, 'S' ); 
    ns = &nameServer;
    bottlingPlant = &plant;
    numVM = numVendingMachines;
    maxStock = maxStockPerFlavour;
    cargo[ 0 ] = 0;
    cargo[ 1 ] = 0;
    cargo[ 2 ] = 0;
    cargo[ 3 ] = 0;
}//constructor

In my destructor class, should I be cleaning up after the pointers? That is, setting them to NULL? or deleting them?

i.e

Truck::~Truck()
{
    printer = NULL; // or should this be delete printer?
    ns = NULL;
    bottlingPlant = NULL;
    // anything else? or is it fine to leave the pointers the way they are?
}//destructor

Thank you for any help, just want to get in to a good habit of creating proper destructors.

Upvotes: 2

Views: 950

Answers (2)

pyrospade
pyrospade

Reputation: 8078

Since your pointers are not being allocated from within the class, neither delete nor NULL would be right here. Since the pointers are being passed from outside the class, leave them alone.

In fact, you are passing in references then converting them into pointers within your constructor. That doesn't seem necessary. Probably better to use them as references internally. Really depends on your use case. If you wanted to use pointers, probably a good idea to have your constructor accept pointers instead. Explicit is better than implicit.

Upvotes: 3

Ned Batchelder
Ned Batchelder

Reputation: 375484

When you store pointers in your object, you need to have a clear understand of who owns the memory they point to. If your class is the owner, then the destructor has to deallocate the memory, or you'll have a leak. If your class is not the owner, then you must not deallocate the memory.

Setting the point to NULL is unnecessary, the important thing is to properly handle the memory itself.

A simpler way to manage pointers is to use a smart pointer class, which will handle this for you automatically.

Upvotes: 7

Related Questions