CODEWITHSUNDEEP
c++classgraphdictionary
BlamKiwi
BlamKiwi

Reputation: 2193

Issues with unordered_map

I'm trying to implement various data structures and algorithms for learning purposes.

At the moment I'm trying to implement a Graph class template but I'm having issues with trying to use STL unordered_map (and consequently priority_queue in the future).

Basically what happens at the moment is, for some reason, the template types don't match up when trying to initialize the vertex map within the graph. From what I understand, since I only plan to use key types from the native C++ types, as long as my value type is a pointer I shouldn't need to do any extra work apart from a copy constructor for my custom vertex class. The default comparer/hasher should suffice. But it doesn't and the error I'm receiving is kind of incomprehensible.

The error:

Error   1   error C2679: binary '=' : no operator found which takes a right-hand operand of type 'std::unordered_map<T,graph<T>::vertex *,std::hash<int>,std::equal_to<_Kty>,std::allocator<std::pair<const _Kty,_Ty>>>' (or there is no acceptable conversion)

The code:

#include "stdafx.h"
#include <iostream>
#include <vector>
#include <unordered_map>
#include <numeric>
#include <functional>

using namespace std;
class vertex;
template <class T>
class graph {
public:

graph() { verts = unordered_map<T, vertex*>(); }
~graph() {
    for each(auto v in verts)
        delete(v);
    delete(verts); 
}
private:
unordered_map<T, vertex*> verts;

// --- Inner Classes ---

struct path {
    vertex *dest;
    double cost;

    path(vertex *d = nullptr, double c = 0.0) : dest(d) : cost(c) {}

    inline int compare(const path& p) {
        auto other = p.cost;

        return cost < other ? -1 :
            cost > other ? 1 : 0;
    }
};

struct edge {
    vertex *dest;
    double cost;

    edge(vertex *d = nullptr, double c = 0.0) : dest(d) : cost(c) {}
};

class vertex {
public:
    // Vertex relationships
    T name;
    vector<edge>* adj;

    // Path Finding Information
    double distance;
    vertex *prev;
    int scratch;

    void reset_path_finding() {
        distance = double.infinity();
        prev = nullptr;
        scratch = 0;
    }

    vertex(T name = default(T)) : name(name) : adj(new vector<edge>) : 
        distance(double.infinity()) : prev(nullptr) : scratch(0) {}
    vertex(const vertex& v) {
        name = v.name;
        adj = v.adj;
        distance = v.distance;
        prev = v.prev;
        scratch = v.scratch;
    }
    ~vertex() { delete(adj); }
private:
};
};

int main()
{
graph<int> myGraph = graph<int>();

cout << "Press any key to continue..." << endl;
int x;
cin >> x;
return 0;
}

Upvotes: 0

Views: 622

Answers (1)

Mike Seymour
Mike Seymour

Reputation: 254431

The first problem is that you use the nested class graph::vertex before declaring it. Further confusion is caused because you've declared class vertex outside graph, so the compiler initially thinks you mean that class. You could declare vertex near the start of graph:

template <class T>
class graph {
    class vertex;
private:
    // and so on
};

There are several other syntax errors, which should be obvious if you look at the lines referred to by the error messages. The syntax for a range-based for loop is

for (auto v : verts)  // not for each(auto v in verts)

This gives you key-value pairs, so to delete the vertex, you need

delete v.second;

Better still, change verts into unordered_map<T, vertex>, containing objects rather than pointers, and it will manage all its memory automatically - you won't need a destructor at all.

The syntax for a value-initialised temporary is

T()  // not default(T)

Clauses in a constructor's initialiser list are separated by commas, not colons:

path(vertex *d = nullptr, double c = 0.0) : dest(d) , cost(c) {}
                                                    ^ not :

A double with infinite value is

std::numeric_limits<double>::infinity() // not double.infinity()

for which you need to in include <limits>.

verts doesn't need deleting in the destructor, since you don't new it. Nor does it need assigning from a default-constructed temporary in the constructor, since it's just been default-constructed.

There are a few places where you are making life difficult for yourself through unnecessary use of pointers and new. Try to avoid new except when you really need it; and learn about RAII, especially the use of smart pointers and containers, for when you do.

Upvotes: 3

Related Questions