0

I am trying to implement hashing in C++ from scratch. Everything seems to be fine except the output. I am implementing hashing using chaining. This style of hashing uses a linked list for handling collisions. I used an array of linked lists of the type HashNode which I defined inside the HashTable class.

#include <bits/stdc++.h>
using namespace  std;

template<class K,class V>
class HashTable{
    class HashNode{
        friend class HashTable;
    public:
        K key;
        V val;
        HashNode(K key, V val){
            this->key = key;
            this->val = val;
        }
        K getKey(){
            return this->key;
        }
        V getVal(){
            return this->val;
        }
    };

    int cap;
    list<HashNode>* ptr = new list<HashNode>[cap];
public:
    //constructor
    HashTable(int x){
        cap = x;    
    }

    //hash function
    hash<K> hashIt;
    int getHash(K k){
        return hashIt(k)%cap;
    }

    //adding pair
    void addPair(K k, V v){
        int index = getHash(k);
        bool found = false;
        auto bucket = *(ptr + index);
        for(auto x = bucket.begin();x!=bucket.end();x++){
            if((*x).key == k){
                (*x).val = v;
                found = true;
                break;
            }
        }
        if(!found){
            bucket.push_back(HashNode(k,v));
        }
    }

    //display function
    void display(){
        for(int i=0;i<cap;i++){
            cout<<"\n Bucket " + i<<" =>"<<endl;
            auto bucket  = *(ptr + i);
            for(auto x = bucket.begin();x!=bucket.end();x++ ){
                cout<<(*x).getKey()<<" : "<<(*x).getVal()<<endl;
            }
        }
    }
};
int main(){
    HashTable<string,int> H(13);
    H.addPair("IND",20);
    H.addPair("PAK",10);
    H.display();
}

However, when I run the program, I get the following output

 Bucket  =>
 Bucket  =>
Bucket  =>
ucket  =>
cket  =>
ket  =>
et  =>
t  =>
  =>
 =>
 => =>
=> =>
> =>

It would be helpful if anyone could point out the mistake.

Varun Dalal
  • 39
  • 7

2 Answers2

2

One mistake is that

cout<<"\n Bucket " + i<<" =>"<<endl;

should be

cout << "\n Bucket " << i << " =>" << endl;

Another mistake is here

    auto bucket = *(ptr + index);
    for(auto x = bucket.begin();x!=bucket.end();x++){
        if((*x).key == k){
            (*x).val = v;
            found = true;
            break;
        }
    }
    if(!found){
        bucket.push_back(HashNode(k,v));
    }

This code copies the bucket and then inserts the node in the bucket copy, not in the original bucket in the array. This is why your hash table stays empty even after you insert items.

Here's the code rewritten to avoid this

    auto bucket = ptr + index;
    for (auto x = bucket->begin(); x!=bucket->end(); ++x) {
        if (x->key == k) {
            x->val = v;
            found = true;
            break;
        }
    }
    if (!found) {
        bucket->push_back(HashNode(k, v));
    }

In this code bucket is a pointer so there is no copy of the bucket made (you could acheive the same effect using a reference).

You have the same issue in your printing routine, you copy the buckets before printing them. This doesn't stop it working but it is inefficient and you should fix it in the same manner as above.

john
  • 85,011
  • 4
  • 57
  • 81
  • This is more a comment then an answer...edit: ah, you're just quickly answering to get it out there and are just gradually updating the answer while you are debugging. – JHBonarius Feb 06 '21 at 20:18
1

Please don't literally convert Java to C++. C++ is a very different language. Look at this:

#include <vector>
#include <functional>
#include <algorithm>
#include <iostream>

template<class K, class V>
class HashTable {
    struct HashNode {
        K key;
        V val;
    };

    std::vector<std::vector<HashNode>> buckets;

    //hash function
    int getBucketIdx(K const& k) const {
        return std::hash<K>{}(k) % buckets.size();
    }
public:
    //constructor
    HashTable(int cap) : buckets(cap) {}

    //adding pair
    void addPair(K const& k, V const& v) {
        auto& bucket = buckets[getBucketIdx(k)];
        auto const loc = std::find_if(begin(bucket), end(bucket),
            [&](HashNode const& hn) { return hn.key == k; });
        if (loc != end(bucket)) {
            loc->val = v;
        } else {
            bucket.emplace_back(HashNode{k, v});
        }
    }

    //display function
    void display() const {
        for(int i = 0; i < buckets.size(); ++i){
            std::cout << "Bucket " << i << " =>\n";
            for(auto const& hn : buckets[i]){
                std::cout << hn.key
                 << " : " << hn.val << '\n';
            }
        }
    }
};
int main() {
    HashTable<std::string, int> H(13);
    H.addPair("IND", 20);
    H.addPair("PAK", 10);
    H.display();
}
JHBonarius
  • 10,824
  • 3
  • 22
  • 41
  • Hey @JHBonarius , Thanks for this code. I was thinking the same that things would be a lot easier if we used vectors instead of arrays – Varun Dalal Feb 06 '21 at 21:21
  • can you please tell me about the constructor of class HashTable that you used here. The part where you initialize buckets using the constructor (after the colon). What is this process called? – Varun Dalal Feb 06 '21 at 21:29
  • @varundalai the problem is not that you use 'an array', but that you dynamically allocate memory (and never clean it up, causing memory leaking away). We rarely use raw allocation in modern c++. A vector internally is a dynamically allocated array. It does all the work for you. – JHBonarius Feb 06 '21 at 22:09
  • `std:: vector` has a number of constructor overloads. The one I'm using initializes the vector with `cap` elements. The colon notation is the initializer list. It's the way how you should initialize member variables in c++. [Read this](https://en.cppreference.com/w/cpp/language/constructor) – JHBonarius Feb 06 '21 at 22:12