patterncppMinor
Implementation of Dijkstra's algorithm to find the shortest path in graph
Viewed 0 times
paththedijkstragraphshortestalgorithmfindimplementation
Problem
This is my implementation of Dijkstra's algorithm in C++.
Please clarify
Please clarify
- Whether the class design is proper or if it needs to be improved.
- I am just hard coding the vertex based on the priority of the vertex. Is there is any other way to implement it?
- Please provide some suggestions to optimize the code.
class Vertex
{
public:
int m_id;
unsigned int m_cost;
bool m_isVisited;
std::map m_edgeList; // map to store neighbour nodes and cost to reach them
void findMinDistance(std::vector&);
Vertex(int p_id)
{
m_id= p_id;
if(p_id == 0)
{
m_cost=0;
}
else
{
m_cost=UINT_MAX; // Assigning Infinite cost
}
m_isVisited=false;
}
Vertex()
{
}
};
void Vertex::findMinDistance(std::vector &p_vertex)
{
m_isVisited= true;
// Looping throgh the neighbour list and update cost
for(std::map::iterator iter = m_edgeList.begin();
iter!=m_edgeList.end(); iter++)
{
//If cost of picked neighbour is greater than current_vertex+new cost then replace cost
if ( p_vertex.at(iter->first).m_isVisited == false && p_vertex.at(iter->first).m_cost > m_cost+(iter->second))
{
p_vertex.at(iter->first).m_cost=m_cost+iter->second;
std::coutfirst).m_id;
std::coutfirst).m_cost;
std::coutfirst).m_cost;
}
}
}
int main()
{
std::vector l_vertex;
for(int i=0; i::iterator iter = l_vertex.begin(); iter != l_vertex.end(); iter++)
{
(*iter).findMinDistance(l_vertex);
}
for(int i=0;i<6;i++)
std::cout<<"\n\nshortest "<<i<<" "<<l_vertex.at(i).m_cost;
return 0;
}Solution
This is a bad idea:
You are conflating two things into a single class. The concept of a vertex and the concept of performing Dijkstra's algorithm on the node. You should separate the two issues into their own classes.
The main problem with this technique is that you need to reset the graph before use. ie. you can not run your algorithm twice in a row. You must reset the state of the whole graph before it can be re-used. Look up the
You really want a constructor that does no initialization?
The POD members will be left in an undefined state.
Your have
Use the new C++ range based for:
Overall I don't think you are actually implement Dijkstra's algorithm.
Dijkstra algorithm
The algorithm should look like this (this is pseudo code).
The classic part is the
class Vertex
{
public:
int m_id;
>>> unsigned int m_cost;
>>> bool m_isVisited;You are conflating two things into a single class. The concept of a vertex and the concept of performing Dijkstra's algorithm on the node. You should separate the two issues into their own classes.
The main problem with this technique is that you need to reset the graph before use. ie. you can not run your algorithm twice in a row. You must reset the state of the whole graph before it can be re-used. Look up the
visitor pattern.You really want a constructor that does no initialization?
Vertex()
{
}The POD members will be left in an undefined state.
Your have
void findMinDistance(std::vector&); in the vertex? Find min distance to where?Use the new C++ range based for:
for(std::map::iterator iter = m_edgeList.begin();
iter!=m_edgeList.end(); iter++)
// can be rewritten as:
for(auto& edge: m_edgeList)Overall I don't think you are actually implement Dijkstra's algorithm.
Dijkstra algorithm
The algorithm should look like this (this is pseudo code).
The classic part is the
frontier list (I called it boundryList) and the set of vertex we have found the shortest route to.RouteCostObject findRout(Graph& g, Vertex& start, Vectex& end)
{
OrderedListByDistance boundryList;
SetOfVertex foundVertixes;
boundryList.push_back([start, 0, []]);
while(!boundryList.empty())
{
// Remove the top items from the list.
(vertex, cost, route) = boundryList.top(); // Note: boundryList is sorted
boundryList.pop();
// If we have reached the end.
// This is the shortest route.
if (vertex == end)
{ return [cost, route];
}
// Check if we have already found the shortest distance
// to this vertex.
if (foundVertixes.find(vertex) != vertex.end())
{
continue;
}
foundVertixes.insert(vertex);
// Add all edges to the boundary list.
for(auto& edge: vertex.edges)
{
boundryList.push_back([edge.dst, cost + edge.cost, route + edge]);
}
}
// No route from start to end
return [infinity, []];
}Code Snippets
class Vertex
{
public:
int m_id;
>>> unsigned int m_cost;
>>> bool m_isVisited;Vertex()
{
}for(std::map<int,unsigned int>::iterator iter = m_edgeList.begin();
iter!=m_edgeList.end(); iter++)
// can be rewritten as:
for(auto& edge: m_edgeList)RouteCostObject findRout(Graph& g, Vertex& start, Vectex& end)
{
OrderedListByDistance boundryList;
SetOfVertex foundVertixes;
boundryList.push_back([start, 0, []]);
while(!boundryList.empty())
{
// Remove the top items from the list.
(vertex, cost, route) = boundryList.top(); // Note: boundryList is sorted
boundryList.pop();
// If we have reached the end.
// This is the shortest route.
if (vertex == end)
{ return [cost, route];
}
// Check if we have already found the shortest distance
// to this vertex.
if (foundVertixes.find(vertex) != vertex.end())
{
continue;
}
foundVertixes.insert(vertex);
// Add all edges to the boundary list.
for(auto& edge: vertex.edges)
{
boundryList.push_back([edge.dst, cost + edge.cost, route + edge]);
}
}
// No route from start to end
return [infinity, []];
}Context
StackExchange Code Review Q#77225, answer score: 6
Revisions (0)
No revisions yet.