review kruskal: some refactoring + todos

min_spanning_tree/kruskal/kruskal.cpp

@@ -2,16 +2,12 @@
 //  Authors: Georǵi Kocharyan
 
 #include <iostream>
-#include <cstdio>
 #include <vector>
-#include <list>
-#include <queue>
 #include <algorithm>
-#include <string>
-#include <functional>
-#include "../../weighted_graph.h"
 #include <tuple>
 
+#include "../../weighted_graph.h"
+
 
  struct key_compare
  {  
@@ -21,9 +17,18 @@
    }  
  };
 
-void next_edge(WeightedGraph const & G, std::vector<std::list<int>> & elements, std::vector<int> component) {
+
-    
+struct Edge
-}
+{
+  // todo: use type alias
+  int from;
+  int to;
+  double weight;
+
+  bool operator<(Edge const & other) const {
+    return weight < other.weight;
+  }
+};
 
 void kruskal(WeightedGraph const & G) {
     // preprocessing: remove all double edges except the minimal ones
@@ -33,7 +38,7 @@ void kruskal(WeightedGraph const & G) {
 
     // preprocessing: create a vector of lists tracking the elements of the components
 
-    std::vector<std::list<int>> elements;
+    std::vector<std::vector<int>> elements;
     elements.resize(H.num_nodes());
     for (int i = 0; i < H.num_nodes(); i++)
     {
@@ -52,53 +57,49 @@ void kruskal(WeightedGraph const & G) {
     // preprocessing: create a vector containing all edges in O(m)
     // then sort them according to their weight
 
+    // todo: use std::vector<Edge>
     std::vector<std::tuple<int, int, double>> edges;
-    edges.resize(H.num_edges());
+    edges.reserve(H.num_edges());
-    int count = 0;
     for (int i = 0; i < H.num_nodes(); i++)
     {
         for (const auto& j : H.adjList(i))
         {
-            edges[count] = std::make_tuple(i,j.first,j.second);
+            edges.emplace_back(i,j.first,j.second);
-            count++;
         }
     }
-    std::sort(edges.begin(), edges.end(),key_compare());
+    std::sort(edges.begin(), edges.end(), key_compare());
 
     for (const auto& edge : edges)
     {
         // check if edge connects two of the same component
-        if (!(component[std::get<0>(edge)]==component[std::get<1>(edge)]))
+        if (component[std::get<0>(edge)] != component[std::get<1>(edge)])
         {
             // output edge
             std::cout << std::get<0>(edge) << "-" << std::get<1>(edge) << "\t" << std::get<2>(edge) << std::endl;
-            total_weight = total_weight + std::get<2>(edge);
+            total_weight += std::get<2>(edge);
             // make the components of each node the same
             // the larger component absorbs the second to guarantee O(mlogn) runtime
+            // todo: avoid code duplication by moving out stuff from if-else
+            int moved;
+            int movedto;
             if (elements[component[std::get<0>(edge)]].size() >= elements[component[std::get<1>(edge)]].size())
             {
                 // move all elements of second component to first
-                int moved = component[std::get<1>(edge)];
+                moved = component[std::get<1>(edge)];
-                int movedto = component[std::get<0>(edge)];
+                movedto = component[std::get<0>(edge)];
-                for (const auto& node : (elements[moved]))
-                {
-                    elements[movedto].push_back(node);
-                    component[node] = movedto;
-                }
-                elements[moved].clear();
             }
             else
             {
                 // move all elements of first component to second
-                int moved = component[std::get<0>(edge)];
+                moved = component[std::get<0>(edge)];
-                int movedto = component[std::get<1>(edge)];
+                movedto = component[std::get<1>(edge)];
-                for (const auto& node : (elements[moved]))
-                {
-                    elements[movedto].push_back(node);
-                    component[node] = movedto;
-                }
-                elements[moved].clear();
             }
+            for (const auto& node : (elements[moved]))
+            {
+              elements[movedto].push_back(node);
+              component[node] = movedto;
+            }
+            elements[moved].clear();
         }
 
     }

weighted_graph.h

@@ -7,23 +7,24 @@
 
 
 struct Node {
+  // todo: introduce neighbor struct and use it instead of std::pair
   std::list<std::pair<int, double>> neighbours;
   Node() = default;
-  Node(std::list<std::pair<int, double>> neighbours);
+  explicit Node(std::list<std::pair<int, double>> neighbours);
   void add_edge(int node_id, double weight);
-  int deg() const;
+  [[nodiscard]] int deg() const;
 };
 
 class WeightedGraph {
 public:
-  WeightedGraph(size_t num_nodes);
+  explicit WeightedGraph(size_t num_nodes);
   void add_edge(int from, int to, double weight);
-  std::list<std::pair<int, double>> adjList(int node_id) const;
+  [[nodiscard]] std::list<std::pair<int, double>> adjList(int node_id) const;
-  size_t num_nodes() const;
+  [[nodiscard]] size_t num_nodes() const;
-  int num_edges() const;
+  [[nodiscard]] int num_edges() const;
-  int deg(int v) const;
+  [[nodiscard]] int deg(int v) const;
-  std::pair<int,double> min_neighbour(int node_id) const;
+  [[nodiscard]] std::pair<int,double> min_neighbour(int node_id) const;
-  WeightedGraph remove_parallel() const;
+  [[nodiscard]] WeightedGraph remove_parallel() const;
 
 private:
   std::vector<Node> nodes;