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

CLRS Implementation of BFS and DFS in C++

Submitted by: @import:stackexchange-codereview··
0
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

#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_H


GraphAlgorithms.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_H


GraphAlgorithms.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.

Context

StackExchange Code Review Q#148191, answer score: 2

Revisions (0)

No revisions yet.