Clean up object-graph implementation

cpp/algorithms/graphs/object/graph.cpp

@@ -51,15 +51,15 @@ int main (const int argc, const char * argv[])
   // Test finding a path between two nodes using BFS
   auto path = bfsGraph.PathBFS(
       bfsGraph.GetNodeCopy(1), bfsGraph.GetNodeCopy(7)
-      );
+  );
   // If we were returned an empty path, it doesn't exist
   if (path.empty()) std::cout << "No valid path found!\n";
   else {
     // If we were returned a path, print it
-    std::cout << "\nValid path from " << path.front()->number
+    std::cout << "\nValid path from " << path.front().number
-              << " to " << path.back()->number << ": ";
+              << " to " << path.back().number << ": ";
     for (const auto &node : path) {
-      std::cout << node->number << " ";
+      std::cout << node.number << " ";
     }
     std::cout << std::endl;
   }
@@ -109,12 +109,12 @@ int main (const int argc, const char * argv[])
   // +  This is because the node is visited after all other nodes are finished
   std::vector<Node> order =
       topologicalGraph.TopologicalSort(topologicalGraph.GetNodeCopy(6));
-  std::cout << "\n\nTopological order: ";
+  std::cout << "\nTopological order: ";
   while (!order.empty()) {
     std::cout << order.back().number << " ";
     order.pop_back();
   }
-  std::cout << std::endl;
+  std::cout << std::endl << std::endl;
 
   // If we want the topological order to match what is seen in the book
   // + We have to initialize the graph carefully to get this result -
@@ -132,10 +132,10 @@ int main (const int argc, const char * argv[])
       }
   );
   auto order2 = topologicalGraph2.TopologicalSort(*topologicalGraph2.NodeBegin());
-  std::cout << "\n\nTopological order: ";
+  std::cout << "\nTopological order: ";
   while (!order2.empty()) {
     std::cout << order2.back().number << " ";
     order2.pop_back();
   }
   std::cout << std::endl;
-}
+}

cpp/algorithms/graphs/object/lib-graph.cpp

@@ -12,54 +12,55 @@
 
 void Graph::BFS(const Node& startNode) const
 {
-  // Track the nodes we have discovered
+  // Track the nodes we have discovered by their Color
-  //  TODO: Do this at the end to maintain the state instead of at beginning?
   for (const auto &node : nodes_) {
     node.color = White;
+    // Track distance from the startNode
     node.distance = 0;
+    // Track predecessor using node that discovers this node
+    // + If this is the startNode, predecessor remains nullptr
     node.predecessor = nullptr;
   }
 
   // Create a queue to visit discovered nodes in FIFO order
-  std::queue<Node> visitQueue;
+  std::queue<const Node *> visitQueue;
 
   // Mark the startNode as in progress until we finish checking adjacent nodes
   startNode.color = Gray;
-//  startNode.distance = 0;
-//  startNode.predecessor = nullptr;
 
   // Visit the startNode
-  visitQueue.push(startNode);
+  visitQueue.push(&startNode);
 
   // Continue to visit nodes until there are none left in the graph
   while (!visitQueue.empty()) {
     // Remove thisNode from the visitQueue, storing its vertex locally
-    Node thisNode = visitQueue.front();
+    const Node * thisNode = visitQueue.front();
     visitQueue.pop();
-    std::cout << "Visiting node " << thisNode.number << std::endl;
+    std::cout << "Visiting node " << thisNode->number << std::endl;
 
     // Check if we have already discovered all the adjacentNodes to thisNode
-    for (const auto &adjacent : thisNode.adjacent) {
+    for (const auto &adjacent : thisNode->adjacent) {
       if (GetNode(adjacent).color == White) {
         std::cout << "Found undiscovered adjacentNode: " << adjacent << "\n";
         // Mark the adjacent node as in progress
         GetNode(adjacent).color = Gray;
-        GetNode(adjacent).distance = thisNode.distance + 1;
+        GetNode(adjacent).distance = thisNode->distance + 1;
-        GetNode(adjacent).predecessor =
+        GetNode(adjacent).predecessor = &GetNode(thisNode->number);
-            const_cast<Node *>(&GetNode(thisNode.number));
 
         // Add the discovered node the the visitQueue
-        visitQueue.push(GetNode(adjacent));
+        visitQueue.push(&GetNode(adjacent));
       }
     }
     // We are finished with this node and the adjacent nodes; Mark it discovered
-    GetNode(thisNode.number).color = Black;
+    GetNode(thisNode->number).color = Black;
   }
 }
 
-std::deque<const Node *> Graph::PathBFS(const Node &start, const Node &finish) const
+std::deque<Node> Graph::PathBFS(const Node &start, const Node &finish) const
 {
-  std::deque<const Node *> path;
+  // Store the path as copies of each node
+  // + If the caller modifies these, it will not impact the graph's data
+  std::deque<Node> path;
 
   BFS(start);
   const Node * next = finish.predecessor;
@@ -69,12 +70,14 @@ std::deque<const Node *> Graph::PathBFS(const Node &start, const Node &finish) c
     if (*next == Node(start)) isValid = true;
 
     // Add the node to the path as we check each node
-    path.push_front(next);
+    // + Use emplace_front to call the Node copy constructor
+    path.emplace_front(*next);
 
     // Move to the next node
     next = next->predecessor;
   } while (next != nullptr);
-  path.push_back(new Node(finish));
+  // Use emplace_back to call Node copy constructor
+  path.emplace_back(finish);
 
   // If we never found a valid path, erase all contents of the path
   if (!isValid) path.erase(path.begin(), path.end());
@@ -108,12 +111,11 @@ void Graph::DFS(const Node &startNode) const
   for (const auto &node : nodes_) node.color = White;
   int time = 0;
 
-  Node begin = startNode;
   auto startIter = std::find(nodes_.begin(), nodes_.end(),
                              Node(startNode.number, {})
   );
 
-  // Visit each node in the graph
+  // beginning at startNode, visit each node in the graph until we reach the end
   while (startIter != nodes_.end()) {
     std::cout << "Visiting node " << startIter->number << std::endl;
     // If the startIter is undiscovered, visit it
@@ -124,9 +126,12 @@ void Graph::DFS(const Node &startNode) const
     }
     startIter++;
   }
+
+  // Once we reach the last node, check the beginning for unchecked nodes
   startIter = nodes_.begin();
 
-  while (! (*startIter == startNode)) {
+  // Once we reach the initial startNode, we have checked all nodes
+  while (*startIter != startNode) {
     std::cout << "Visiting node " << startIter->number << std::endl;
     // If the startIter is undiscovered, visit it
     if (startIter->color == White) {
@@ -143,6 +148,7 @@ void Graph::DFSVisit(int &time, const Node& startNode) const
   startNode.color = Gray;
   time++;
   startNode.discoveryFinish.first = time;
+
   // Check the adjacent nodes of the startNode
   for (const auto &adjacent : startNode.adjacent) {
     auto iter = std::find(nodes_.begin(), nodes_.end(),

cpp/algorithms/graphs/object/lib-graph.hpp

@@ -56,7 +56,7 @@ public:
   mutable int distance = 0;
   // Used in BFS to represent the parent node that discovered this node
   // + If we use this node as the starting point, this will remain a nullptr
-  mutable Node *predecessor = nullptr;
+  mutable const Node *predecessor = nullptr;
 
   // Create a pair to track discovery / finish time when using DFS
   // + Discovery time is the iteration the node is first discovered
@@ -67,9 +67,12 @@ public:
   // Define a comparator for std::sort
   // + This will help to sort nodes by finished time after traversal
   static bool FinishedSort(const Node &node1, const Node &node2)
-      { return node1.discoveryFinish.second < node2.discoveryFinish.second;}
+  { return node1.discoveryFinish.second < node2.discoveryFinish.second;}
-  // Define operator== for std::find
+
+  // Define operator== for std::find; And comparisons between nodes
   bool operator==(const Node &b) const { return this->number == b.number;}
+  // Define an operator!= for comparing nodes for inequality
+  bool operator!=(const Node &b) const { return this->number != b.number;}
 };
 
 
@@ -83,29 +86,32 @@ public:
 
   // Breadth First Search
   void BFS(const Node& startNode) const;
-  std::deque<const Node *> PathBFS(const Node &start, const Node &finish) const;
+  std::deque<Node> PathBFS(const Node &start, const Node &finish) const;
 
 
   // Depth First Search
   void DFS() const;
+  // An alternate DFS that checks each node of the graph beginning at startNode
   void DFS(const Node &startNode) const;
+  // Visit function is used in both versions of DFS
   void DFSVisit(int &time, const Node& startNode) const;
   // Topological sort, using DFS
   std::vector<Node> TopologicalSort(const Node &startNode) const;
 
 
   // Returns a copy of a node with the number i within the graph
+  // + This uses the private, non-const accessor GetNode()
   inline Node GetNodeCopy(int i) { return GetNode(i);}
   // Return a constant iterator for reading node values
-  inline std::vector<Node>::const_iterator NodeBegin() { return nodes_.begin();}
+  inline std::vector<Node>::const_iterator NodeBegin() { return nodes_.cbegin();}
 
 private:
   // A non-const accessor for direct access to a node with the number value i
   inline Node & GetNode(int i)
-    { return *std::find(nodes_.begin(), nodes_.end(), Node(i, {}));}
+  { return *std::find(nodes_.begin(), nodes_.end(), Node(i, {}));}
   // For use with const member functions to access mutable values
   inline const Node & GetNode(int i) const
-    { return *std::find(nodes_.begin(), nodes_.end(), Node(i, {}));}
+  { return *std::find(nodes_.begin(), nodes_.end(), Node(i, {}));}
 
   std::vector<Node> nodes_;
 };