2

I am trying to rehash a table by deleting old table and creating a new bigger table with same contents. I created a reHash function, but this function gives memory leaks, causing the program to crash when the function is executed. I can't find my error.

void HashMap::reHash()
{
int OldCapacity = cap;
cap = cap * 2 + 1; //set new capacity
Node** newHashTable = new Node*[cap]; //create a temporary table to hold info
for (int i = 0; i < cap; i++)
{
    newHashTable[i] = nullptr;
}

const Node* n;
//fill in the new temp table with old info
for (int i = 0; i < OldCapacity; ++i)
{
    n = HashTable[i];
    while (n != nullptr)
    {
        //initialize new node
        Node* nod = new Node;
        nod->key = n->key;
        nod->value = n->value;
        nod->next = nullptr;
        Node*& bucket = newHashTable[default_hash_function(n->key)/cap];
        nod->next = bucket;
        bucket = nod;
        n = n->next;
    }
}

// delete the links 
for (int i = 0; i < OldCapacity; ++i)
{
    Node *curr = HashTable[i];
    Node *next;
    while (curr != nullptr)
    {
        next = curr->next;
        delete curr;
        curr = next;
    }
}
HashTable = newHashTable;
}
Sargis Plus Plus
  • 45
  • 1
  • 3
  • 11
  • 1
    You can always look at the implementation of `std::unordered_map::rehash()` for inspiration. http://www.cplusplus.com/reference/unordered_map/unordered_map/rehash/ – Adam Nov 18 '13 at 00:38

1 Answers1

5

Your fundamental leak is this:

HashTable = newHashTable;

You never deleted the old pointer-array. It should be this:

delete [] HashTable;
HashTable = newHashTable;

You also aren't computing the modulo of your hash function correctly for your table size in the reassignment, which would be devastating to your hash table.

This:

Node*& bucket = newHashTable[default_hash_function(tmp->key) / cap];

should be this:

Node*& bucket = newHashTable[default_hash_function(tmp->key) % cap];
// note modulo operator -------------------------------------^

Rehash Sans-Allocations

Honestly, none of the dynamic allocations are needed in this except allocating the new bed. You can use the existing nodes by moving them from the old bed to the new one.

void HashMap::reHash()
{
    int OldCapacity = cap;
    cap = cap * 2 + 1;

    // allocate new bed. note: uses () to value-initialize nullptr entries
    Node** newHashTable = new Node*[cap]();

    //fill in the new temp table with old info
    for (int i = 0; i < OldCapacity; ++i)
    {
        Node *n = HashTable[i];
        while (n != nullptr)
        {
            // advance n *before* moving node to new hash bed
            Node *tmp = n;
            n = n->next;

            // find the proper collision list pointer.
            Node*& bucket = newHashTable[default_hash_function(tmp->key) % cap];
            tmp->next = bucket;
            bucket = tmp;
        }
    }

    delete [] HashTable;
    HashTable = newHashTable;
}
WhozCraig
  • 65,258
  • 11
  • 75
  • 141