patterncppMinor
Graph vertex class implementation with adjacency lists
Viewed 0 times
graphvertexwithadjacencylistsimplementationclass
Problem
I wrote an implementation of a graph with each vertex storing a
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
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
An idea of implementation:
with:
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.