patterncppMinor
CLRS Implementation of BFS and DFS in C++
Viewed 0 times
bfsdfsclrsandimplementation
Problem
I tried to implement BFS and DFS from Introduction to Algorithms by Cormen. Any suggestions on how to improve my code?
GraphStructures.h
GraphAlgorithms.h
GraphAlgorithms.cpp
```
#include "stdafx.h"
#include "GraphAlgorit
GraphStructures.h
#pragma once
#ifndef GRAPH_STRUCTURES_H
#define GRAPH_STRUCTURES_H
#include
#include
#include
#include
#include
enum class Status { UNDISCOVERED, DISCOVERED, PROCESSED };
enum { INF = std::numeric_limits::max() };
struct Vertex
{
int id;
Status status;
size_t distance;
Vertex* parent;
size_t start;
size_t finish;
Vertex(int vertex) : id{ vertex } {};
};
struct Graph
{
std::vector vertex = std::vector(250);
//Adjacency List
std::vector> adj = std::vector> (250);
//add edges from a file
void add_edge(std::string filename)
{
Vertex* obj_u = nullptr, *obj_v = nullptr;
std::ifstream file_in(filename);
std::string line;
while (getline(file_in, line))
{
int u, v;
std::istringstream iss(line);
iss >> u;
iss >> v;
if (vertex[u] == nullptr)
{
obj_u = new Vertex(u);
vertex[u] = obj_u;
}
if (vertex[v] == nullptr)
{
obj_v = new Vertex(v);
vertex[v] = obj_v;
}
//undirected graph where u->v and v->u
adj[u].push_back(vertex[v]);
adj[v].push_back(vertex[u]);
}
}
~Graph()
{
for each (auto& var in vertex)
{
if (var)
delete var;
}
}
};
#endif // !GRAPH_STRUCTURES_HGraphAlgorithms.h
#pragma once
#ifndef GRAPH_ALGORITHMS_H
#define GRAPH_ALGORITHMS_H
#include "Graphstructures.h"
#include
void BFS(const Graph&, const int);
void print_path(const Graph& G, const int, const int);
void DFS(const Graph&);
void visit(const Graph&, const int);
#endif // !GRAPH_ALGORITHMS_HGraphAlgorithms.cpp
```
#include "stdafx.h"
#include "GraphAlgorit
Solution
Rule of 0/3/5
As your Graph class possess pointers and allocate their memory you should follow the rule of 0 or 5.
Rule of three: If a class requires a user-defined destructor, a user-defined copy constructor, or a user-defined copy assignment operator, it almost certainly requires all three.
As C++11 implements the move semantics, you should also define the move constructor and move assignment constructor. (rule of 5)
You can avoid the rule of 5 by using smart pointers instead, making it the rule of 0.
SOLID Principle
You probably did this for the example only, but your Graph class breaks the first SOLID principle:
Single responsibility principle: The single responsibility principle states that every module or class should have responsibility over a single part of the functionality provided by the software.
As your Graph class is a struct that only store the vertexes data, it shouldn't handle the whole file opening thing. This part should be done by another class that could open any file for any purpose. Making your Graph opening the file makes it dependent on this storage system while it could be more generic, it is also the first step to copy-pasted code.
C++11
You are using c++11 features correctly and that's worth being noted. Besides that, you declare and define a global variable in GraphAlgorithms.cpp.
size_t time = 0;
This is probably a good sign that you need to encapsulate your functions in a class with a time attribute.
As your Graph class possess pointers and allocate their memory you should follow the rule of 0 or 5.
Rule of three: If a class requires a user-defined destructor, a user-defined copy constructor, or a user-defined copy assignment operator, it almost certainly requires all three.
As C++11 implements the move semantics, you should also define the move constructor and move assignment constructor. (rule of 5)
You can avoid the rule of 5 by using smart pointers instead, making it the rule of 0.
SOLID Principle
You probably did this for the example only, but your Graph class breaks the first SOLID principle:
Single responsibility principle: The single responsibility principle states that every module or class should have responsibility over a single part of the functionality provided by the software.
As your Graph class is a struct that only store the vertexes data, it shouldn't handle the whole file opening thing. This part should be done by another class that could open any file for any purpose. Making your Graph opening the file makes it dependent on this storage system while it could be more generic, it is also the first step to copy-pasted code.
C++11
You are using c++11 features correctly and that's worth being noted. Besides that, you declare and define a global variable in GraphAlgorithms.cpp.
size_t time = 0;
This is probably a good sign that you need to encapsulate your functions in a class with a time attribute.
Context
StackExchange Code Review Q#148191, answer score: 2
Revisions (0)
No revisions yet.