What should be in a proper destructor?

Question

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.

It depends on what's in your constructor(s). See: [What is The Rule of Three?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — Mysticial, Jul 26 '12 at 01:17
Thank you for the reference. I will definitely take a look. I have added the constructor for you to take a look at as well. — tf.rz, Jul 26 '12 at 01:19
After reading Ned's answer I think you got it, but I'm saying is you may want to check your pointers before usage because there is a possibility that `Printers`'s owner deleted it and `Truck` doesn't know, and that will cause a crash. — shengy, Jul 26 '12 at 01:38
@shengy Printer doesn't have an owner though. Printer gets instantiated first. :) — tf.rz, Jul 26 '12 at 01:45
@tf.rz you passed `Printer` in by reference, so there must be a place you define `Printer`, if `Printer` is deleted, in this case, maybe out of scope, and you still use it in `Trunk`, it will cause a crash. — shengy, Jul 26 '12 at 01:54
@shengy ahh I see what you mean. I'll be careful about that. Thanks! — tf.rz, Jul 26 '12 at 02:04
@tf.rz, Your header guard is violating the fact that identifiers beginning with an underscore, followed by a capital letter, are reserved for the implementation. Before you happen to choose this alternative, starting it with two underscores is the same deal. — chris, Jul 26 '12 at 02:22
@chris good point, thank you, I will change that part. If I have no underscores and just TRUCK_H__, is that alright? — tf.rz, Jul 26 '12 at 02:38

score 7 · Accepted Answer · answered Jul 26 '12 at 01:18

7

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.

answered Jul 26 '12 at 01:18

Ned Batchelder

364,293
75
561
662

1

Use smart pointers and other forms of RAII consistently and you'll never have to write your own destructor - the one generated automatically by the compiler will be sufficient. – Mark Ransom Jul 26 '12 at 01:20
Ah, thank you. I did not have anything in there, it was simply undefined. I will leave it that way. =) – tf.rz Jul 26 '12 at 01:31
@NedBatchelder Does that mean in his case he must not delete the three pointers in his destructor? Cause his three pointers are all pointing to vars which `Truck` don't owns. – shengy Jul 26 '12 at 01:36
@Mark Random: slight overstatement. There are generic "Scope Guards" like in Alexandrescu's Loki, but sometimes it's easier to write your own classes for RAII management, so you're factoring and localising your destructors rather than eliminating them. Other times it can be needless complication to shoehorn things like on-destruction logging into an RAII mechanism. – Tony Delroy Jul 26 '12 at 01:51

score 3 · Answer 2 · answered Jul 26 '12 at 01:24

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.

Thank you for your answer! I was under some constraints and would definitely have done it a little differently but I will leave the destructor empty based on your answer and Ned's answer. — tf.rz, Jul 26 '12 at 01:32

What should be in a proper destructor?

2 Answers2