unordered_set example compiler error (hash and equivalence function error possibily)

Question

I am trying to compile this code in C++11 and it gives me compile errors:

#include <iostream> 
#include <unordered_set>

using namespace std; 

class Point{
  int x,y; 
 public: 
  Point(int a=0, int b=0): x(a), y(b) {}; 
  int getX(void){ return x;} 
  int getY(void){ return y;} 
};


class monkeyHash{
 public: 
    size_t operator()(const Point & xy){
        size_t h1 = std::hash<int>()(xy.getX()); 
        size_t h2 = std::hash<int>()(xy.getY());
        return (h1 ^ (h2<<1));  
 }
};

struct monkeyEquals{
 bool operator()(const Point & lhs, const Point & rhs){
     return (lhs.getX() == rhs.getX()) && (lhs.getY() == rhs.getY()); 
 }
};

int main(){
  unordered_set<Point,monkeyHash,monkeyEquals> visited; 
  visited.insert(Point(0,0));   

}

The error message I get is really long so I will summarize it:

In member function ‘size_t monkeyHash::operator()(const Point&)’:
monkey.cpp:26:50: error: passing ‘const Point’ as ‘this’ argument of ‘int    Point::getX()’ discards qualifiers [-fpermissive]
monkey.cpp:27:50: error: passing ‘const Point’ as ‘this’ argument of ‘int Point::getY()’ discards qualifiers [-fpermissive]
monkey.cpp: In member function ‘bool monkeyEquals::operator()(const Point&, const Point&)’:

And it also seems to complain about my hash function. Is the way that I am hashing incorrect? I am still very new to template, c++11 and STL, so sorry if the issue is very noobish.

Answer 1

Declare those methods const:

int getX(void) const { return x; }
int getY(void) const { return y; }

Using const like this, tells the compiler that the method will not change the class instance. Only then you can call it on a const Point object.

In C++11 you might also want to mark them noexcept, (e.g. int getX() const noexcept { return x; }, because they don't throw any exceptions. If you mark them noexcept, the compiler might have the chance to optimize your code a bit more.

The same goes for the size_t monkeyHash::operator()(const Point &) and bool monkeyEquals::operator()(const Point &, const Point &)methods.

PS: You can probably get rid of the monkeyEquals class if you instead overload operator== in class Point {}:

inline bool operator==(const Point & rhs) const noexcept
{ return (x == rhs.x) && (y == rhs.y); }

You can also override operator!=:

inline bool operator!=(const Point & rhs) const noexcept
{ return (x != rhs.x) || (y != rhs.y); }

See this wikipedia article on how to write a better custom hash function in namespace std.

If you take the advice in this post-scriptum, you can declare your container like this:

unordered_set<Point> visited;