Adjacency-list graph implementation in C++ (redone from scratch)

I had posted here before, but I decided that the approach used earlier - where every Vertex stores a vector of pointers to vertices it's adjacent to - was just not worth the hassle (read segfaults) it was causing me. (I'm relatively inexperienced with pointers as of now.)

Besides, a Vertex isn't more than a wrapped long in most of my use cases (implementing basic graph algorithms from CLRS), so passing by value makes sense too. (See end, though.)

Here's the code, please critique this.
(I know that the WeightT parameter is just lying there, I'll write the relevant methods later.)

```
#ifndef _GRAPH_H
#define _GRAPH_H

#include
#include
#include
#include
#include
#include
#include

enum Color { WHITE, GRAY, BLACK, RED, NONE }; //for DFS

//TODO remove this later
//using namespace std;

template class Vertex {
template friend class Graph;
//but is it even required?

private:
PayloadT payload;
Color color;

public:
Vertex() : payload(PayloadT()), color(WHITE) { }
Vertex(PayloadT p) : payload(p), color(WHITE) { }

Vertex& operator=(Vertex& other) {
swap(*this, other);
return *this;
}

bool operator==(const Vertex& other) const {
return (payload == other.get_payload()) &&
(color == other.get_color());
}

PayloadT get_payload() const { return payload; }
PayloadT set_payload(PayloadT p) { payload = p; }

Color get_color() const { return color; }
Color set_color(Color c) { color = c; }
};

//For the std::unordered_map
template struct VertexHasher {
std::size_t operator()(const Vertex& v) const {
return std::hash()(v.get_payload());
}
};

template class Graph {
using Vert = Vertex;
//using VertPtr = shared_ptr;
using VertList = std::vector;
using AdjList = std::unordered_map>;

private:
AdjLis

Here are some observations that may help you improve this code.

Fix the obvious errors

The Graph::set_color() routine says that it returns a Color but it doesn't return anything. The obvious fix is to simply add return color; to that function. Alternatively, change the signature to return void. The same problem exists with set_payload.

Iterate over const references where possible

In the to_gv_dot() routine, the for loops should iterate over const references instead of forcing temporary copies. In other words, change the code from this:

for (auto j : i.second) {

to this:

for (auto const &j : i.second) {

Use defaults when sensible

The Vertex operator= is essentially the same as that which would have been generated by the compiler. To reduce the chance of error, it can be simply eliminated. If you wish to make it explicit that this is being done, use this:

Vertex &operator=(Vertex& ) = default;

Also note that it is not necessary to write Vertex in this context because the compiler can infer that from the context. Omitting it makes it easer for humans to read and understand and eliminates one possibility for error.

Consider adding debug-friendly methods

The Vertex class can be more easily debugged by adding a few lines of code. First this code just below the Color enum:

static const char *colorname[] = { "white", "gray", "black", "red", "none" };

Then these class members in the Vertex class:

const char *get_color_name() const { return colorname[color]; }
    friend std::ostream &operator<<(std::ostream &out, const Vertex &v) {
        return out << "{ " << v.get_payload() << ", " << v.get_color_name() 
            << " }\n";
    }

The get_color_name() routine can also be used to enhance the existing output for dot. In the to_gv_dot() routine, the line after the first for can be this:

stm << "\t" << i.first.get_payload() << "[style=filled,fillcolor=" 
        << i.first.get_color_name() << "];\n";

Have public methods return user-centric values

Graph has a get_size() function which seems like it ought to return the number of vertices or the number of edges, but it does neither. Instead it returns the number of unique nodes which are sources of directed edges to other nodes. It would make more sense for this function to return something such as number of nodes or number of edges which would make sense to the user of the graph.

Fix rm_vertex

The rm_vertex() method of Graph purports to remove a vertex, but in fact, it only eliminates part of the internal data structure. If one has the graph

a -> b -> c

and then calls rm_vertex(b) the resulting graph will be

a -> b

even though the user just asked to delete that vertex. That can't be right.

Consider the user when designing the interface

There is no way to simultaneously set the contents and the color when constructing a Vertex. Also, once a Vertex is created and inserted into a Graph, there's no way to get it back or refer to it.

Avoid using a leading underscore for items in global namespace

As you can read in this answer, global names that begin with an underscore are "reserved to the implementation;" that is, they are for your compiler rather than for you.

Consider providing iterators

An obvious thing to do with a graph is to iterate over edges or nodes. It would be nice for your interface to provide this.

Decide if nodes that differ only in color are actually different

Right now, if two nodes differ only in color, the operator== for the Vertex class reports that they are not equal. However, if two nodes differ only in color, they will be shown on the dot graph as only a single node.

StackExchange Code Review Q#67999, answer score: 3