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

Implementation of Prim's algorithm in C++

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

Problem

Could anyone comment on what could be done better and if I made any mistakes?

```
#include
#include
#include

using namespace std;

typedef pair, int> Edge; //a, b, length
// A---------B
// length

vector prims_algo(vector> graph, int number_of_nodes){
vector unvisited;
vector visited;
vector result;

//mark first as visited and mark the rest as unvisited
for (int i = 1; i edges_with_lengths;
//put all edges (with their lengths) from nodes that are in visited
for(auto node : visited) {
for (int sec_node = 0; sec_node 0 && find(unvisited.begin(), unvisited.end(), sec_node) != unvisited.end()) {
//add if there is connection and second node is not visited yet
Edge e = make_pair(make_pair(node, sec_node), graph[node][sec_node]);
edges_with_lengths.push_back(e);
}
}
}

//find the shortest edge
pair, int> the_shortest;
the_shortest = edges_with_lengths.front();
for(auto i: edges_with_lengths){
if(the_shortest.second > i.second)
the_shortest = i;
}

//add the shortest path to the result
result.push_back(the_shortest);

//remove a node that the shortest edge leads to
unvisited.erase(remove(unvisited.begin(), unvisited.end(), the_shortest.first.second), unvisited.end());

//mark this node as visited
visited.push_back(the_shortest.first.second);
};
return result;
}

int main() {
/*
2 3
(0)--(1)--(2)
| / \ |
6| 8/ \5 |7
| / \ |
(3)-------(4)
9 */
vector> graph =
{{{0, 2, 0, 6, 0},
{2, 0, 3, 8, 5},
{0, 3, 0, 0, 7},
{6, 8, 0, 0, 9},
{0, 5, 7, 9, 0},
}};

vector result = prims_algo(graph, 5);
for(auto i:result){
cout

Solution

Code looks OK.

I would suggest you to do some class instead of Edge:

//typedef pair, int> Edge; //a, b, length
struct Edge{
   int a;
   int b;
   int length;
};


Also I suggest to have some predefined type for vector

using EdgeContainer = vector;


You have duplication near //find the shortest edge, you probably need Edge there

Here the_shortest is probably not initialized. Also you probably might use something from ` (but I will do it in same way as you).

//find the shortest edge
    pair, int> the_shortest;
    the_shortest = edges_with_lengths.front();
    for(auto i: edges_with_lengths){
        if(the_shortest.second > i.second)
            the_shortest = i;
    }


Another probably
micro optimization would be to write ++i in loops:

//for (int i = 1; i < number_of_nodes; i++)
for (int i = 1; i < number_of_nodes; ++i){
   // ...
}


Also in
C++11 main() does not need to return 0;`.

Code Snippets

//typedef pair<pair<int, int>, int> Edge; //a, b, length
struct Edge{
   int a;
   int b;
   int length;
};
using EdgeContainer = vector<Edge>;
//find the shortest edge
    pair<pair<int, int>, int> the_shortest;
    the_shortest = edges_with_lengths.front();
    for(auto i: edges_with_lengths){
        if(the_shortest.second > i.second)
            the_shortest = i;
    }
//for (int i = 1; i < number_of_nodes; i++)
for (int i = 1; i < number_of_nodes; ++i){
   // ...
}

Context

StackExchange Code Review Q#126617, answer score: 7

Revisions (0)

No revisions yet.