1

I'm trying to store Objects in unordered map, using custom build Hash Function, below is the code,

#include <iostream>
#include <unordered_map>
#include <string>

//Class that I want to store in unordered map
class Tree{
public:
    int val;
    std::pair<int,int> location;

    Tree(int val,std::pair<int,int> location){
        this->location = location;
        this->val = val;}
};


//I guess this operator is internally used by the unordered map to compare the Values in the maps. 
//This function returns True only when the val of both the nodes are equal.

bool operator ==(const Tree& node1, const Tree& node2){
    return node1.val == node2.val;}

//Custom build Hash Function for assigning Keys to the Values.
class HashFunction{
public:
    size_t operator()(const Tree& node) const{
        std::hash<int> Hash;
        return Hash(node.val);
    }
};

//Created a class dictionary using the std::unordered_map to store the objects.
class dictionary{
public:
    std::unordered_map<Tree,Tree*,HashFunction> dict;

    void append(Tree node){
        this->dict[node] = &node;}

    Tree* get(Tree node){
        return this->dict[node];}

};


int main(){

    Tree obj1(1,std::make_pair(1,6));
    Tree obj2(2,std::make_pair(2,5));
    Tree obj(2,std::make_pair(3,4));

    dictionary dict;
    dict.append(obj1);
    dict.append(obj2);


    std::cout<<"#################"<<std::endl;  
    std::cout<<dict.get(obj)->location.first<<std::endl;    

}

The result obtained is '3' (as in obj.val), instead of '2' (as in obj2.val).

I created three objects of Tree class obj1, obj2 and obj in the main function. obj1 and obj2 are stored in the dictionary, and obj is used to check the matching object in the dictionary. Since the Hash Function uses the object's val to create Key, hence both obj2 and obj will have same Keys, but when I'm trying to access the dictionary with obj as input, the dictionary should return obj2 instead I'm getting obj which is not in the dictionary, I don't understand why it's happening. Any suggestions would be appreciated. Thanks in advance :)

  • 3
    `this->dict[node] = &node;` will result in undefined behavior, as your unordered_map will store pointers to local variables that expire once the function is left. What is your intention? – Jodocus Nov 15 '19 at 07:39
  • You're needlessly making a lot of copies of `Tree` objects, which (at least partly) caused this issue. Try using [pass by (const) reference](https://stackoverflow.com/questions/2139224/how-to-pass-objects-to-functions-in-c) whenever appropriate instead. – Sander De Dycker Nov 15 '19 at 08:07

1 Answers1

0

In dictionary::append, you insert as the value a pointer to a local variable (node) :

this->dict[node] = &node;

This pointer will no longer be valid once the function ends.

Trying to dereference that pointer at a later point results in undefined behavior. This undefined behavior manifests itself (in your case) by accessing the wrong object (specifically, the parameter to the dictionary::get function). It could have been worse.

To fix it, you could simply change the function to :

void append(Tree& node){
    this->dict[node] = &node;}

You would still be reliant upon the objects in main to keep existing, but at least it should do what you appear to want.

Sander De Dycker
  • 16,053
  • 1
  • 35
  • 40