patterncppMinor
Eulerian Path algorithm
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
Some some parts of the code are in Portuguese.
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:
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
Again there's a Stack Overflow Q&A pointing out in detail why you shouldn't do so.
Your function
is inconsistent with
One would expect a signature like
Also your implementation always returning
If you actually don't need that function, just omit it.
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
but never check that
This will lead to calling undefined behavior if
The C++ standard provides the
Though in your case it is probably even better to use
While it is legit to use
For example this piece of code
can be made more efficiently readable as
Also
can be simplified as
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:
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.
You should mark parameters or member functions as
- 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 - Do not use
using namespace std;
Again there's a Stack Overflow Q&A pointing out in detail why you shouldn't do so.
- 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.
- 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 arraysint 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.- 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.- 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.- Use
boolreturn 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());
}- 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.
- Use
constwhenever 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.