How can I use an unordered_set with a custom struct?

Question

I want to use an unordered_set with a custom struct. In my case, the custom struct represents a 2D point in an euclidean plane. I know that one should define a hash function and comparator operator and I have done so as you can see in my code below:

struct Point {
    int X;
    int Y;

    Point() : X(0), Y(0) {};
    Point(const int& x, const int& y) : X(x), Y(y) {};
    Point(const IPoint& other){
        X = other.X;
        Y = other.Y;
    };

    Point& operator=(const Point& other) {
        X = other.X;
        Y = other.Y;
        return *this;
    };

    bool operator==(const Point& other) {
        if (X == other.X && Y == other.Y)
            return true;
        return false;
    };

    bool operator<(const Point& other) {
        if (X < other.X )
            return true;
        else if (X == other.X && Y == other.Y)
            return true;

        return false;
    };

    size_t operator()(const Point& pointToHash) const {
        size_t hash = pointToHash.X + 10 * pointToHash.Y;
        return hash;
    };
};

However, I'm getting the error below, if I define the set as follows:

unordered_set<Point> mySet;

Error C2280 'std::hash<_Kty>::hash(const std::hash<_Kty> &)': attempting to reference a deleted function

What am I missing?

Blasphemous

Answer 1

The second template parameter to std::unordered_set is the type to use for hashing. and will default to std::hash<Point> in your case, which doesn't exist. So you can use std::unordered_set<Point,Point> if the hasher is the same type.

Alternatively if you do not want to specify the hasher, define a specialization of std::hash for Point and either get rid of the member function and implement the hashing in the body of your specialization's operator(), or call the member function from the std::hash specialization.

#include <unordered_set>

struct Point {
    int X;
    int Y;

    Point() : X(0), Y(0) {};
    Point(const int& x, const int& y) : X(x), Y(y) {};
    Point(const Point& other){
        X = other.X;
        Y = other.Y;
    };

    Point& operator=(const Point& other) {
        X = other.X;
        Y = other.Y;
        return *this;
    };

    bool operator==(const Point& other) const {
        if (X == other.X && Y == other.Y)
            return true;
        return false;
    };

    bool operator<(const Point& other) {
        if (X < other.X )
            return true;
        else if (X == other.X && Y == other.Y)
            return true;

        return false;
    };

    // this could be moved in to std::hash<Point>::operator()
    size_t operator()(const Point& pointToHash) const noexcept {
        size_t hash = pointToHash.X + 10 * pointToHash.Y;
        return hash;
    };

};

namespace std {
    template<> struct hash<Point>
    {
        std::size_t operator()(const Point& p) const noexcept
        {
            return p(p);
        }
    };
}


int main()
{
    // no need to specify the hasher if std::hash<Point> exists
    std::unordered_set<Point> p;
    return 0;
}

Demo

Legault

Thanks @rmawatson! I've decided to go with the first option - specifying the hash function. Pretty new to all of the new unordered associative structures and I sure have a lot to learn on this. – Blasphemous

Answer 2

I'd like to expand on rmawatson's answer by providing some more tips:

1. For your struct, you neither need to define operator= nor Point(const Point& other), because you (re-)implemented the default behavior.

2. You can streamline operator== by removing the if clause as follows:

   bool operator==(const Point& other) { return X == other.X && Y == other.Y; };
   

3. There is a mistake in your operator<: In the else if clause, you return true if both points are equal. This violates the requirement for a strict weak ordering. Therefore, I recommend to use the following code instead:

   bool operator<(const Point& other) { return X < other.X || (X == other.X && Y < other.Y); };
   

Moreover, since C++11, you can use lambda expressions instead of defining the hash and comparison functions. This way, you don't need to specify any operators for your struct, if you don't need them otherwise. Putting everything together, your code could be written as follows:

struct Point {
    int X, Y;

    Point() : X(0), Y(0) {};
    Point(const int x, const int y) : X(x), Y(y) {};
};

int main() {
    auto hash = [](const Point& p) { return p.X + 10 * p.Y; };
    auto equal = [](const Point& p1, const Point& p2) { return p1.X == p2.X && p1.Y == p2.Y; };
    std::unordered_set<Point, decltype(hash), decltype(equal)> mySet(8, hash, equal);

    return 0;
}

However, as also explained in CJ13's answer, your hash function might not be the best one. Another way to handcraft a hash function is the following:

auto hash = [](const Point& p) { return std::hash<int>()(p.X) * 31 + std::hash<int>()(p.Y); };

The idea for a more general solution to hashing can be found here.

Code on Ideone

Dictionary

Answer 3

While the above solution gets you compiling code, avoid that hash function for points. There's a one dimensional subspace parameterized by b for which all points on the line y = -x/10 + b will have the same hash value. You'd be better off with a 64 bit hash where the top 32 bits are the x coord and the low 32 bits are the y coord (for example). That'd look like

uint64_t hash(Point const & p) const noexcept
{
    return ((uint64_t)p.X)<<32 | (uint64_t)p.Y;
}

Northington