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

Eulerian Path algorithm

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
eulerianalgorithmpath

Problem

I'm doing a project to find the Eulerian path. Cane someone find an example where the algorithm is wrong? The function EulerianPath recursively prints the Eulerian Path.

Some some parts of the code are in Portuguese.

#include 
using namespace std;

class Grafo{
    private:
        int N,A; // N: Vertices; A: Arestas;
        int graph[100][100]; // matriz do grafo
        bool visited[100];
    public:
        Grafo(int graphsize);
        int getArestas();
        void setArestas(int x, int y);
        bool isComplete();
        int getDegree(int v);
        bool isEuler();
        void EulerCicle();
        void setNodes(int k);
        int maxArestas();
        int Goodman();
        void DFS(int);
        void BFS(int);
        /* Dijkstra */
        int minDistance( int*, bool* );
        int printSolution(int dist[], int n, int parent[], int i, int src);
        void Dijkstra( int, int );
        void printPath(int parent[], int j);
        /**/
        void EulerianPath( int src );
};

Grafo::Grafo(int graphsize){
    N = graphsize;
    memset(graph, 0, sizeof(graph));
    memset(visited, false, sizeof(visited));
}

int Grafo::getArestas(){

    return 0;
}

void Grafo::setNodes(int k){
        N = k;
}

void Grafo::setArestas(int x, int y){
        graph[x-1][y-1] = 1;
        A++;
}

int Grafo::maxArestas(){
    return ((N*(N-1))/ 2);
}

int Grafo::getDegree(int v){
        int aux = 0;
        for(int i = 0; i  Q;

    visited[s] = true;
    Q.push(s);

    while( !Q.empty() )
    {
        s = Q.front();
        cout  " > k;
    Grafo G(k);

    while(cin >> x >> y && x != 0 && y != 0 ){
        G.setArestas(x,y);
        G.setArestas(y,x);
    }
    if( G.isEuler() )
        G.EulerianPath( 0 );

    return 0;
}

Solution

A few points:

  1. Do not use #include



As pointed out in detail here, this may get you in trouble and isn't really portable code.

Always just include the standard headers you need like

#include 


  1. Do not use using namespace std;



Again there's a Stack Overflow Q&A pointing out in detail why you shouldn't do so.

  1. Keep getter / setter functions consistent



Your function

int getArestas();


is inconsistent with

void setArestas(int x, int y);


One would expect a signature like

void getArestas(int& x, int& y);


Also your implementation always returning 0 looks weird:

int Grafo::getArestas(){

    return 0;
}


If you actually don't need that function, just omit it.

  1. Check input values for fixed boundaries




someone can help me to find an example to make the algorithm wrong

At least this point may easily break your code.

You introduce a fixed boundary of 100 when declaring your arrays

int graph[100][100]; // matriz do grafo
    bool visited[100];


but never check that x and y are smaller than or equal to 100 before dereferencing the array:

void Grafo::setArestas(int x, int y){
        graph[x-1][y-1] = 1;
        A++;
}


This will lead to calling undefined behavior if x or y are grater than 100.

  1. Do not use raw C-style arrays in C++



The C++ standard provides the std::array class to specify fixed arrays.

Though in your case it is probably even better to use std::vector instead, since the maximum size is taken from input more or less.

  1. Prefer formatted text output to streams in C++



While it is legit to use printf() in C++ code, you should prefer to use the
std::ostream operator<<(std::ostream&, T) because it's more typesafe than using the printf() format string.

  1. Use bool return values efficiently



For example this piece of code

bool Grafo::isEuler(){
    bool aux = true;
    for(int i = 0; i < N; i++){
        if(getDegree(i) % 2 != 0)
        {
            aux = false;
            break;
        }
    }
    return aux;
}


can be made more efficiently readable as

bool Grafo::isEuler(){
    for(int i = 0; i < N; i++){
        if(getDegree(i) % 2 != 0)
        {
            return false;
        }
    }
    return true;
}


Also

bool Grafo::isComplete(){
    if( A == maxArestas())
        return true;
    return false;
}


can be simplified as

bool Grafo::isComplete(){
    return (A == maxArestas());
}


  1. Keep language consistent for naming types, variables and functions




Yes the code have some parts in Portuguese.

You have a weird mix in the style of language used to name types, variables and functions:

class Grafo{ // << Portugese
    private:
        int graph[100][100]; // << English with portugese comment (matriz do grafo)
        bool visited[100]; // << English
    public:
        Grafo(int graphsize); // << Portugese / Egnlish 
        int getArestas();  // << Portugese
        void setArestas(int x, int y);  // << Portugese
        bool isComplete();   // << English
        int getDegree(int v);  // << English
        bool isEuler();  // << English
        void EulerCicle();  // << English
        void setNodes(int k);  // << English
        int maxArestas();   // << Portugese
        int Goodman();   // << English
        void DFS(int);
        void BFS(int);
        /* Dijkstra */
        int minDistance( int*, bool* ); // << English
        int printSolution(int dist[], int n, int parent[], int i, int src);   // << English
        void Dijkstra( int, int );
        void printPath(int parent[], int j); // << English
        /**/
        void EulerianPath( int src ); // << English
};


Choose any language, but keep it consistent please. My personal preference is to use English for naming in code and comments as well.

This will be most widely understood by anyone else who's reading your code.

  1. Use const whenever appropriate



You should mark parameters or member functions as const whenever it is appropriate, i.e. these aren't changing the parameters or class' current state:

class Grafo{ // << Portugese
        int getArestas() const;
        bool isComplete() const;
        int getDegree(int v) const;
        bool isEuler() const;
        // etc. ...
};

Code Snippets

#include <queue>
int getArestas();
void setArestas(int x, int y);
void getArestas(int& x, int& y);
int Grafo::getArestas(){

    return 0;
}

Context

StackExchange Code Review Q#153017, answer score: 4

Revisions (0)

No revisions yet.