patterncppMinor
Depth First Search and Breadth First Search in C++
Viewed 0 times
depthsearchbreadthfirstand
Problem
I am trying to learn DFS and BFS. However, I just want to make sure that I am not doing anything wrong. Would you please determine if the BFS and DFS functions are correct?
#include
#include
#include
#include
using namespace std;
class Graph
{
private:
int num_of_vertices;
vector *Adj;
public:
Graph(int V);
void addEdge(int from, int to);
void BFS(int Start);
void DFS(int Start);
};
Graph::Graph(int V)
{
this->num_of_vertices = V;
Adj = new vector[V];
}
void Graph::addEdge(int from, int to)
{
Adj[from].push_back(to);
}
void Graph::BFS(int Start)
{
bool* visited = new bool[this->num_of_vertices]();
queue queue;
queue.push(Start);
vector::iterator i;
cout num_of_vertices]();
stack stack;
stack.push(Start);
vector::iterator i;
cout << "DFS: ";
while(!stack.empty())
{
int top = stack.top();
cout << top << " ";
stack.pop();
visited[top] = true;
for(i = Adj[top].begin(); i != Adj[top].end(); i++)
{
if (!visited[*i])
stack.push(*i);
}
}
cout << endl;
}Solution
The implementation of the
-
In the constructor,
-
The same is true for the
Actually, there is a much easier way to deal with this issue: do not use pointers and dynamic memory allocation. I do not see any point in
One more thing: you should try to keep the scope of variables as narrow as possible. There is no need to declare the
The last thing:
BFS and DFS algorithms themselves seems correct(but it is a good practice to write unit-tests to make sure that it works as intended, so you should do it). However, your code in general is not correct. It definitely leaks memory:-
In the constructor,
Adj is allocated: Adj = new vector[V];. However, it is never deleted. You can do it in a destructor.-
The same is true for the
visited array(you should delete it at the end of the DFS and BFS member-functions).Actually, there is a much easier way to deal with this issue: do not use pointers and dynamic memory allocation. I do not see any point in
visited being allocated dynamically. Using an std::vector is much better(std::vector visited(num_of_vertices)). The same holds true for the Adj member-variable. One more thing: you should try to keep the scope of variables as narrow as possible. There is no need to declare the
vector::iterator i at the beginning of the function. You can do it just inside the for loop where it is used. The last thing:
using namespace std; is a bad practice. It pollutes the global namespace.Context
StackExchange Code Review Q#82476, answer score: 8
Revisions (0)
No revisions yet.