-3

I have a HashTable class. The purpose of this is to practice how to produce Hash Table. My current issue right now is the destructor. I should be seeing on the console, this: enter image description here

But instead i see this:

enter image description here

I've ran gdb ./app which gives me this: enter image description here

#include <iostream>
#include "HashTable.h"

using namespace std;

int main(){
 HashTable table;
 table.initTable();
 table.addNode("Software");
 table.addNode("Engineering");
 table.addNode("Art");
 table.addNode("Programming");
 table.addNode("Miscellanous");
 table.displayByIndex();
 return 0;
}

HashTable Header:

#ifndef _HASHTABLE_H_
#define _HASHTABLE_H_

#include <iostream>
#include "Hash.h"

class HashTable{
private:
    static const int TABLESIZE = 5;
    Hash* head;
public:
    HashTable();
    ~HashTable();

    void initTable();
    int hashKey(char*);
    int quadProbing(int,int);
    int hashKeyWithProbing(char*);
    bool isEmpty();
    void addNode(char*);
    void displayByIndex();
    void searchByKey(char*);

};#endif

Here it my Hash Table CPP, i included the Constructor/Destructor and init functions. The other functions arent included since i dont utilize the 'new' keyword. If its needed i will include it [hope to make this thread small]

#include "HashTable.h"
#include <cstring>

HashTable::HashTable(){
  head = new Hash[TABLESIZE];
}
HashTable::~HashTable(){
  Hash* destList = NULL;
   for(int i = 0; i< TABLESIZE; i++){
     destList = &head[i];
     while(destList != NULL){
        head = head->getNext();
        delete destList;
        destList = head;
     }
   }
   head = NULL;
   delete [] head;
}
void HashTable::initTable(){
  for(int i = 0; i < TABLESIZE; i++){
    Hash *traverseHeadArray = &head[i];
    traverseHeadArray->setKey("EMPTY");
    traverseHeadArray->setNext(NULL);
    traverseHeadArray = NULL;
  }
} 

int HashTable::hashKey(char *tempKey){
 //Find the asc value for the string
//add them together
//modules by the total table size
//then return the index (remainder using modules)
//Index is were that key will be stored
 int index = 0;
 for(int i = 0; i < strlen(tempKey); i++){
    index += tempKey[i];
 }
  return index % TABLESIZE;
 }//DONE
int HashTable::quadProbing(int index, int counter){
  return (index + counter*counter) % TABLESIZE;
}//DONE
int HashTable::hashKeyWithProbing(char* key){
 int index = hashKey(key);
 int count = 1;

 while(head[index].getKey() != key && head[index].getKey() != "EMPTY"){
    index = quadProbing(index, count);
    count++;
 }
 return index;
}//DONE

void HashTable::addNode(char* tempValue){
 Hash* newNode = new Hash;
 newNode->setKey(tempValue);
 newNode->setNext(NULL);
 int index = hashKey(tempValue);
 int counter = 1;

 while(head[index].getKey() != tempValue && head[index].getKey() !="EMPTY")
  {
    index = quadProbing(index, counter);
    counter++;
  }

  Hash* traverse = &head[index];
 if(isEmpty() || traverse->getKey() == "EMPTY"){
    traverse->setKey(newNode->getKey());
    traverse->setNext(newNode->getNext());
 }else{
    while(traverse->getNext() != NULL)
        traverse = traverse->getNext();
    traverse->setNext(newNode);
 }
 traverse = NULL;  
 delete newNode;
}
void HashTable::displayByIndex(){
 for(int i = 0; i < TABLESIZE; i++){
    Hash *traverse = &head[i];
    std::cout << "----------------------------------------" << std::endl;
    std::cout << "INDEX: " << i << std::endl; 
    while(traverse != NULL){
        std::cout << "Key: " << traverse->getKey() << std::endl;
        traverse = traverse->getNext();
    }
    std::cout << "----------------------------------------" << std::endl;
    traverse = NULL;
  }
 }
 bool HashTable::isEmpty(){
  return (head == NULL);
 }
 void HashTable::searchByKey(char* key){
   int index = hashKeyWithProbing(key);

   if(isEmpty())
    std::cout << "Empty Bucket\n";
   else{
    Hash* traverse = &head[index];
    while(traverse != NULL && traverse->getKey() != "EMPTY"){
        std::cout << "TOPIC KEY: " << traverse->getKey() << std::endl;
        traverse = traverse->getNext();
    }
     traverse = NULL;
   }
  }

Here is my Hash Class header and CPP files:

#ifndef _HASH_H_
#define _HASH_H_

class Hash{
 private:
    char* key;
    Hash* next;

 public:
    Hash();
    ~Hash();

    void setKey(char*);
    void setNext(Hash* const);
    char* getKey();
    Hash* getNext();

  };#endif



#include "Hash.h"
#include <iostream>

Hash::Hash(){
 key = NULL;
 next = NULL;
}
Hash::~Hash(){
// TODO Destructor
}

void Hash::setKey(char* tempKey){
 this->key = tempKey;
}
void Hash::setNext(Hash* const tempNext){
 this->next = tempNext;
}
char* Hash::getKey(){
 return key;
}
Hash* Hash::getNext(){
  return next;
}
  • 1
    Please paste your code as text into the question. Read about how to create a [mcve] as well. – Retired Ninja May 27 '18 at 20:24
  • @RetiredNinja apologies, i though it be faster but then i realize it be easier for everyone to copy and paste the code. Thanks for the heads up! – Jonathan Vazquez May 27 '18 at 20:31
  • Can you provide your testings, output and desired output – Dennis Vash May 27 '18 at 20:32
  • @DennisVash i've include main.cpp , i'm simply just trying to add keys into each index using linked list. The file compile but i get a invalid pointer on gdb and segfault. i just want to output what each list within a specific index. – Jonathan Vazquez May 27 '18 at 20:39
  • `head = NULL; delete [] head;` - You are calling `delete[]` on a null pointer? – UnholySheep May 27 '18 at 20:39
  • @UnholySheep Yeah i was shown that its safer to make it NULL then delete. I'm going to retrace my steps and find the article i read about it, and link it here. – Jonathan Vazquez May 27 '18 at 20:41
  • 1
    Calling `delete[]` on a null pointer is essentially a no-op. Are you sure the article didn't say to `delete` first and *then* set the pointer to null? (Though even that would only be required if the pointer is still accessible afterwards) – UnholySheep May 27 '18 at 20:42
  • @UnholySheep okay so i lied, i reference my notes again and i misunderstood it. They suggested to do it when initializing an object. Hash* head = NULL; head = new Hash; not before deletion. – Jonathan Vazquez May 27 '18 at 20:44
  • Well, that is also incorrect, but off-topic for the problem at hand. We still require a [mcve] and what your debugging efforts have shown as problem (candidates) – UnholySheep May 27 '18 at 20:47
  • @JonathanVazquez add `addNode(..)` implementations and of other methods you using – Dennis Vash May 27 '18 at 20:54
  • @DennisVash I've added more information. There is a few functions that i didnt include since they're mainly basic arithmetic functions. – Jonathan Vazquez May 27 '18 at 20:59
  • Please include everything, I just want to run your code so it much easier to answer – Dennis Vash May 27 '18 at 21:01
  • @DennisVash i didnt place everything since some users mentioned the Minimal, Complete, and Verifiable example article. Anyways i went ahead and included everything. – Jonathan Vazquez May 27 '18 at 21:09
  • The **Complete** part means you don't just miss out chunks of code. You can simplify the code to remove things (that makes it **Minimal**) but don't just fail to show us parts of the code if you didn't minimise it. Your classes don't have copy constructors, that is usually a sign of a bug. Stop using names like `_HASH_H_` and `_HASHTABLE_H_` for macros, those are [reserved names](http://stackoverflow.com/q/228783/981959), just use `HASH_H` and `HASHTABLE_H` and stop copying bad habits from other people. – Jonathan Wakely May 27 '18 at 22:05
  • @JonathanWakely is there any articles or sites you recommend i read, perhaps a book that provides better programming approach? I'm not sure whats consider good or bad since my uni tends to teach all aspects of CS even methodology thats consider deprecated. – Jonathan Vazquez May 27 '18 at 23:04
  • [tag:c++] has a list of recommended books – Jonathan Wakely May 27 '18 at 23:06
  • Do you have warnings turned on in your compiler settings? You're comparing strings using `!=`which compares the address and not the content. If you're going to use C++ you'd be better off with `std::string`. – Retired Ninja May 28 '18 at 08:12

1 Answers1

1

In your code, you allocating only the getNext() nodes, so you should delete only them, (you tried to delete &head[i] which wasn't allocated at all). There is still an exception for double freeing some node, you should debug and follow the stack for figuring it out.

HashTable::~HashTable(){
    for(int i = 0; i< TABLE_SIZE; i++){
        Hash* node = head[i].getNext();
        while(node != nullptr){
            Hash* temp = nullptr;
            if (node -> getNext()) {
                temp = node->getNext();
            }
            delete node;
            node= temp;
        }
    }
    delete[] head;
}

Output:

----------------------------------------
INDEX: 0
Key: Art
----------------------------------------
----------------------------------------
INDEX: 1
Key: Engineering
----------------------------------------
----------------------------------------
INDEX: 2
Key: Miscellanous
----------------------------------------
----------------------------------------
INDEX: 3
Key: Software
----------------------------------------
----------------------------------------
INDEX: 4
Key: Programming
----------------------------------------
double free or corruption (fasttop)

Process finished with exit code 134 (interrupted by signal 6: SIGABRT)
Dennis Vash
  • 50,196
  • 9
  • 100
  • 118
  • So i noticed the line `if(!head->getNext()){break;}` , i've added it to my source and i manage to get it compile and running. However, when i try to add more keys into those index, i still get a seg fault., the seg fault is still the same one. I've included everything up above. – Jonathan Vazquez May 27 '18 at 21:15
  • I'm sorry but i'm still getting a segfault. Even after changing it to `if(!head){break;}` – Jonathan Vazquez May 27 '18 at 21:19
  • @JonathanVazquez I suggest rechecking the `addNode` method. – Dennis Vash May 27 '18 at 21:57
  • so in my addNode class, i cant get it to output if i decide to add another `table.addNode("Miscellanous);` from within main. However, i decided to remove the `delete newNode;` command from the addNode function. Now i get it to output the information, necessary, but there's still a seg fault due to the `newNode` not been unallocated. – Jonathan Vazquez May 27 '18 at 23:07