0

I was trying to implement BFS using Vertex classes and BFS class, since I wanted to learn implementation of classes along with algorithms.

vertex.h

#ifndef vertex_H_
#define vertex_H_
#include<vector>


class Vertex{

    private:
         int _data;
         bool _visited;
         std::vector<Vertex> _neighbours;

    public:
         Vertex(int data);
         Vertex();
         void setData(int data);
         int getData();
         bool isVisited();
         void setVisited(bool visited);
         std::vector<Vertex> getNeighbours();
         void setNeighbours(std::vector<Vertex>& neighbours);
         void addNeighbourVertex(Vertex& vertex);
         ~Vertex();
};

#endif

vertex.cpp

#include "vertex.h"


Vertex::Vertex(int data):_data(data){}
Vertex::Vertex(){}
Vertex::~Vertex(){}

void Vertex::setData(int data){
    _data = data;
}


int Vertex::getData(){
    return _data;
}

void Vertex::setVisited(bool visited){
    _visited=visited;
}

bool Vertex::isVisited(){
    return _visited;
}

std::vector<Vertex> Vertex::getNeighbours(){
    return _neighbours;
}

void Vertex::setNeighbours(std::vector<Vertex>& neighbours){
    _neighbours = neighbours;
}

void Vertex::addNeighbourVertex(Vertex& vertex){
    _neighbours.push_back(vertex); 
}

BFS.h

#ifndef BFS_H_
#define BFS_H_
#include "vertex.h"

class BFS{
    public:
        void bfs(Vertex& root);
        
};

#endif

BFS.cpp

#include<iostream>
#include<vector>
#include"vertex.h"
#include<deque>
#include"BFS.h"

void BFS::bfs(Vertex& root)
{  
    std::deque<Vertex> Q;
    root.setVisited(true);
    Q.push_back(root);

    while(!Q.empty())
    {
        Vertex select_node = Q.front();
        Q.pop_front();
        std::cout<<select_node.getData()<<" "<<std::endl;
        for(Vertex node : select_node.getNeighbours())
        {
            if (!node.isVisited())
            {
                node.setVisited(true);
                Q.push_back(node);
            }
        }
        
    }
}

application.cpp

#include<iostream>
#include "BFS.h"
#include "vertex.h"


int main(){

    BFS bfs;

    Vertex v1(1);
    Vertex v2(2);
    Vertex v3(3);
    Vertex v4(4);
    Vertex v5(5);

    v1.addNeighbourVertex(v2);
    v1.addNeighbourVertex(v4);
    v4.addNeighbourVertex(v5);
    v2.addNeighbourVertex(v3);

    bfs.bfs(v1);





    return 0;
}
  • getNeighbours function from vertex class gets the vector of neighbouring vertices of particular vertex.

Problem I am facing is that for graph in application.cpp that I have defined.

vertex 1-> vertices{2,4}

vertex 2-> vertex{3}

vertex 3-> vertex{2}

vertex 4-> vertex{5}

vertex 5-> vertex{4}

expected output is: 1 2 4 3 5

My output : 1 2 4

I would really like some assistance.. there is some thing i am unable to see here, and hence not getting the proper output.

Aditya Srichandan
  • 23
  • 2
  • 4
    My advice is for you to take this as an opportunity to learn how to use a *debugger*. With a debugger you can step through your code statement by statement, jumping into your own functions calls, al while monitoring variables and their values. If there's no warnings (with many extra warnings enabled) when building and there's no obvious problems found in the code, then using a debugger is the way to find and fix problems. – Some programmer dude Aug 24 '20 at 18:04
  • `_visited` is not initialized to false in the constructor – sebastian Aug 24 '20 at 18:48

2 Answers2

3

You are using containers of Vertex. Think about that. Think about shallow copies.

Vertex v1(1);
Vertex v2(2);
Vertex v3(3);
Vertex v4(4);
Vertex v5(5);

So far, so good. You have 5 vertices, each without neighbors.

v1.addNeighbourVertex(v2);
v1.addNeighbourVertex(v4);

Now the _neighbours member of v1 contains copies of v2 and v4.

v4.addNeighbourVertex(v5);
v2.addNeighbourVertex(v3);

Yeah, fine, but the nodes contained in v1's _neighbours member are still neighborless.

bfs.bfs(v1);

This gives you a search of v1 and its "neighbors", namely the neighborless copies of v2 and v4:

1 2 4

If you store pointers to vertices, you'll have better luck.

Beta
  • 96,650
  • 16
  • 149
  • 150
  • Also, it might be a good idea to delete the copy constructor and copy assignment operator of `Vertex`, which would have caused the compiler to give an error when trying to copy into `_neighbours`. – G. Sliepen Aug 24 '20 at 18:49
  • @G.Sliepen: Copying a `Vertex` is not inherently wrong unless|until we decide that it is. Another design might do that and work correctly. This design was based on a misunderstanding of how copies work. If we change the design so that `_neighbors` is a container of `Vertex*`, then the compiler will balk when we try to put a `Vertex` in it. – Beta Aug 24 '20 at 18:54
  • Thanks @Beta, I tried with `std::vector _neighbours` and got the expected result, I stored all the vertices pointers – Aditya Srichandan Aug 25 '20 at 05:40
0

It's happening because you're copying nodes and vectors all over the place. The particular problem here is that you create v1 and v2, then attach v1 to v2 by pushing v2 onto v1._neighbors. This is a copy of v2, not the original! THEN you set the neighbors for v2, but you are setting neighbors NOT for the v2 in v1._neighbors, but for the v2 in main.

The quickest solution is to define the graph upwards in main. But a better solution is to pass around pointers instead of actual instances to avoid copying, this would change all the code.

smorgasborgas
  • 61
  • 6