HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Graph vertex class implementation with adjacency lists

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
graphvertexwithadjacencylistsimplementationclass

Problem

I wrote an implementation of a graph with each vertex storing a std::vector of vertices it's adjacent to.

Please critique this.

```
#ifndef GRAPH_H
#define GRAPH_H

#include
#include
#include

template class Vertex
{
typedef typename std::vector::size_type adjlist_sz;
//typedef typename std::pair*, long> Edge; (weighted)
typedef Vertex* Edge;

public:
Vertex(); //NPC
Vertex(const Vertex& v); //copy ctor
Vertex(const T& payload);

Vertex& operator=(Vertex v); //yes

virtual ~Vertex();
void add_adj(Vertex& v);
void remove_adj(Vertex& v);

const bool is_adj_to(const Vertex& v);

//const W get_edge_weight(const Vertex& v);
//void set_edge_weight(Vertex& v, const W w);

const T get_datum();
void set_datum(const T& new_datum);

//const adjlist_sz degree();
//const adjlist_sz in_degree();
const adjlist_sz out_degree();

inline friend void swap(Vertex& v1, Vertex& v2)
{
using std::swap;
swap(v1.datum, v2.datum);
swap(v1.adj_vertices, v2.adj_vertices);
}

private:
T datum;
std::vector adj_vertices; //Adj-list representation
};

//np ctor
template
Vertex::Vertex() : datum(T()), adj_vertices({})
{
std::cout
Vertex::Vertex(const Vertex& v)
: datum(v.get_datum()), adj_vertices({})
{
std::cout
Vertex::Vertex(const T& payload)
: datum(payload), adj_vertices({})
{
std::cout
Vertex::~Vertex()
{
std::cout ().swap(adj_vertices);
}

//= operator
template
Vertex& Vertex::operator=(Vertex v)
{
swap(*this, v);
return *this;
}

template
void Vertex::add_adj(Vertex& v)
{
adj_vertices.push_back(&v);
}

template
void Vertex::remove_adj(Vertex& v)
{
std::remove(adj_vertices.begin(), adj_vertices.end(), &v);
}

template
const bool Vertex::is_adj_to(const Vertex& v)
{
return std::find(
adj_vertices.begin(), adj_vertices.end(), &v
) != adj_vertices.end();
}

//const W get_edge_weight(co

Solution

Storing raw pointers is fine if they are not associated with ownership: as you have noted, if you translate simply to smart pointers they can start being deleted when you don't want (not to mention handling cycles in the graph).

I think a much simpler solution would be to have the Vertex unaware of its neighbors, and store edge connections in the graph itself. The graph would own a list of Vertex and a list of Edge (= pair of Vertex then, not a single one), and memory management will become simple.

An idea of implementation:

// Not very useful class as it is, you might as well use T directly in Graph
template class Vertex
{
public:
    Vertex(const T& payload);
    const T get_datum();
    void set_datum(const T& new_datum);
private:
    T datum; // Confuse use of 2 terms: "datum" vs "payload"
};

template class Graph
{
public:
    typedef boost::shared_ptr> VertexPtr;
    VertexPtr createVertex(const T& new_datum);
    void addEdge(const VertexPtr& v1, const VertexPtr& v2);
private:
    typedef std::pair Edge;
    std::vector vertices;
    std::vector edges;
};


with:

VertexPtr Graph::createVertex(const T& new_datum)
{
    vertices.push_back(VertexPtr(new Vertex(new_datum)));
}

Code Snippets

// Not very useful class as it is, you might as well use T directly in Graph
template<class T> class Vertex
{
public:
    Vertex(const T& payload);
    const T get_datum();
    void set_datum(const T& new_datum);
private:
    T datum; // Confuse use of 2 terms: "datum" vs "payload"
};

template<class T> class Graph
{
public:
    typedef boost::shared_ptr<Vertex<T>> VertexPtr;
    VertexPtr createVertex(const T& new_datum);
    void addEdge(const VertexPtr& v1, const VertexPtr& v2);
private:
    typedef std::pair<VertexPtr, VertexPtr> Edge;
    std::vector<VertexPtr> vertices;
    std::vector<Edge> edges;
};
VertexPtr Graph::createVertex(const T& new_datum)
{
    vertices.push_back(VertexPtr(new Vertex<T>(new_datum)));
}

Context

StackExchange Code Review Q#64641, answer score: 3

Revisions (0)

No revisions yet.