C++, memory and arrays. Creating my own hashmap for exercise. Unexpected data left in memory?

Question

So i'm trying to create a pretty specific for my needs hashmap for a small project where i'm trying to learn c++. I have the following code:

template<class T>
class HashMap
{
public:
  HashMap();
  virtual ~HashMap();
  void add(T value);
  T get(T *value);
private:
  int hash(T *data);
  T _hashes[26]; //I want a fixed size here
};

template<class T>
HashMap<T>::HashMap()
{
  for(int i = 0; i < 26; i++)
    this->_hashes[i] = T();
}

template<class T>
HashMap<T>::~HashMap()
{
  //Don't really have anything to delete here?
}

template<class T>
int HashMap<T>::hash(T *dat)
{
  //Super simple, just to try things out
  return (long int) dat % 26;
}

template<class T>
T HashMap<T>::get(T *val)
{
  int idx = this->hash(val);
  cout << idx << endl;
  //Probably somewhere here i get my problem
  if(this->_hashes[idx])
    return this->_hashes[idx];
  return T();
}

template<class T>
void HashMap<T>::add(T val)
{
  //Should probably do some check if there's already an element here.
  this->_hashes[this->hash(&val)] = val;
}

The problem im having is that this compiles fine but when I do something like this in my main.cpp:

HashMap<char> a = HashMap<char>();
a.add('h');
a.add('c');
a.add('g');
char *b = new char {'c'};
cout << a.get(b) << endl;
delete b;

It usually returns the id, ie:

4

and an empty line which is just a empty char. (the output of the function is in the get() method), but sometimes it will show me something like this:

18
g

instead of 18 and an empty line. My question is why this happens and how i can prevent it? Does it have something to do with memory not being 'nulled' when it's deleted but just free for other programs to take and then i don't initialise it correctly? Also, if you have the time please point out any mistakes or not so good to do things in the code.

If it's of any interest im using GCC Debian 4.4.5-8 to compile and compile it with g++ -g file.cpp -o file

Thankful for any help!

Answer 1

The behavior you see is normal: if you get a value that you put in your hash, it will be displayed by your main. What is giving you surprising results is your hash function:

return (long int) dat % 26;

This hashes the dat pointer, not the T that dat points to. Try with:

return *dat % 26;

(Or just use a standard std::set.)

Another problem with your code:

T _hashes[26]; //I want a fixed size here   (a

and

this->_hashes = new T[26];                   (b

are incompatible. Either use the fixed array (a) and you don't need to allocate it (b), or use a plain pointer (T *_hashes) and do (b) - I'm surprised your compiler accepts what you have. If you use (a) you don't need anything in the destructor. If you use (b), you need to delete [] in your destructor.

Passing a T* in get but a T in set is a bit strange too.

Answer 2

Here's a more idiomatic c++ implementation:

#include <array>
#include <iostream>
#define MAGIC_NUMBER 26 //rename this to something else preferably

template<class T>
class HashMap
{
    public:
        HashMap();
        virtual ~HashMap(){};
        void add(T value);
        T get(T *value);//potentially confusing that add and get take different types
    private:
        int hash(T *data);
        std::array<T, MAGIC_NUMBER>  _hashes; //I want a fixed size here
};

template<class T>
HashMap<T>::HashMap()
{
    std::fill(_hashes.begin(),_hashes.end(), T());
}

template<class T>
int HashMap<T>::hash(T *dat)
{
    //Super simple, just to try things out
    return (static_cast<int>(*dat)) % MAGIC_NUMBER;//prefer using c++ casts
}

template<class T>
T HashMap<T>::get(T *val) //this is strange, you pass in a pointer and get a non-pointer
{
  int idx = this->hash(val);
  std::cout << idx << std::endl;
  if(this->_hashes[idx])
    return this->_hashes[idx];
  return T();
}

template<class T>
void HashMap<T>::add(T val)
{
  //Should probably do some check if there's already an element here.
  this->_hashes[this->hash(&val)] = val;
}

int main(void){
    HashMap<char> a = HashMap<char>();
    a.add('h');
    a.add('c');
    a.add('g');
    char *b = new char {'c'};
    std::cout << a.get(b) << std::endl;
    delete b;
}

Note you will need to compile with c++0x or c++11 features to get the usage of the std::array class. One of the main benefits of the array class is that you get more safety with the memory allocation than just a plain c-style array.

Right now you might want to reconsider some elements of the design. In particular it is confusing that add and get, have different types. Also this isn't what people generally think about when they hear hashmap, this structure is more like a set.

Also as a coding standards note, if you are prefixing your member variables it's a bit tautological to also use this-> to access them.