CODEWITHSUNDEEP
c++memory-managementmemory-leaksdestructor
sigmasquared
sigmasquared

Reputation: 1

Do I have to use delete to avoid memory leaks? I have used new to allocate the structure instances

The following code is implementing a simple hash function. I have allocated the struct instances with the new operator.

Do I have use the delete operator when the program ends?

If so, how do I do it? [ A delete statement for each of the instances of the struct? Or is there an easier way of doing it? ]

#include <iostream>
#define SIZE 10

using namespace std;

typedef struct myhashtag
{
   int data;
   struct myhashtag * next;
} myhash;
void printhash(myhash array[])
{
// print fn
}

void hash(int data, myhash array[])
{
   int h = data % SIZE;
   myhash * newhash = new myhash;
   newhash->data = data;
   newhash->next = NULL;

   if(array[h].next == NULL)  //first insert
   {
      array[h].next = newhash;
      return;
   }
   myhash * iter = array[h].next;
   while(iter->next != NULL)
   {
      iter = iter->next;
   }
   iter->next = newhash;
   return;
}


int main()
{
   myhash array[SIZE];int i;
   for(i=0; i<SIZE; i++)
   {
      array[i].data = 0;
      array[i].next = NULL;
   }

   while(1)
   {
      cout << "\nSo, what would you like to enter? : ";
      cin >> i;
      hash(i, array);
      printhash(array);
   }

// the deletes go here 

   return 0;
}

Upvotes: 0

Views: 222

Answers (3)

Max Lybbert
Max Lybbert

Reputation: 20039

C++ does require you to manage the lifetime of objects. It's not nearly as hard as many Java or C# books might lead you to believe. But, yes, without using a garbage collector, you need to be sure that delete gets called appropriately.

There are two ways I can imagine doing this for the code you posted. The first is, as others have suggested, to use std::unique_ptr.

The second is to give your struct a destructor. I'm not really clear why you use the classic C typedef struct ... idiom (you do that in C so that you don't have to use the struct keyword as often, but you don't have to use the struct keyword as often in C++). I'll keep it in the code that follows, but I want to flag it as nonidiomatic:

typedef struct myhashtag
{
   int data;
   struct myhashtag * next;

   ~myhashtag()
   {
      if (next) {
         delete next;
      }
   }
} myhash;

(You still need to be sure to delete the head node where you put the comment that "deletes happen here").

However, this assumes that next is correctly assigned to NULL when it's the end of the list. The code you posted does that, but it's only enforced by convention. A slightly more idiomatic approach would be:

struct hash_node {
   int data;
   hash_node* next;

   hash_node(int d, n = NULL) : data(d), next(n) { }

   ~hash_node()
   {
      if (next) {
         delete next;
      }
   }
};

void hash(int data, hash_node* array)
{
   int h = data % SIZE;
   hash_node* newhash = new hash_node(data);

   if(array[h].next == NULL)  //first insert
   {
      array[h].next = newhash;
      return;
   }
   hash_node* iter = array[h].next;
   while(iter->next != NULL)
   {
      iter = iter->next;
   }
   iter->next = newhash;
   return;
}

A much more idiomatic approach would be:

#include <list>
#include <iostream>
using namespace std'

int main()
{
    list<int> list_that_the_code_was_calling_a_hash;
    while(1) {
        cout << "\nSo, what would you like to enter? : ";
        cin >> i;
        list_that_the_code_was_calling_a_hash.push_back(i);
        if (list_that_the_code_was_calling_a_hash.size() > 10) {
            list_that_the_code_was_calling_a_hash.pop_front();
        }
        printhash(list_that_the_code_was_calling_a_hash);
    }
    return 0;
}

And many people would recommend using a std::vector because std::list simply doesn't interact well with your CPU cache. Yes, std::vector does a lot more copying when you muck witht he middle of the container. But it more than makes up for that by being very cache friendly.

Upvotes: 1

Lee Fergusson
Lee Fergusson

Reputation: 19

Yes you should always delete any dynamic memory which you allocate.

You could implement a simple RAII container to handle your memory cleanup. Just create a class to act as a handle which news the memory in its constructor. deletes it inside its destructor and provides some kind of access mechanism. That way any memory is automatically cleaned up at the end of the scope in which the handleis declared in.

Upvotes: 1

manuBriot
manuBriot

Reputation: 2715

In general, you will indeed need to perform a delete (or delete[] for arrays) once for each allocation with new.

If you are on linux, valgrind can help detect memory leaks, by the way.

Recent versions of C++, as well as the Boost library, provided various sorts of smart pointers, which basically will free the memory automatically when the object is no longer referenced.

Lookup std::unique_ptr in your C++ book of choice.

Upvotes: 3

Related Questions