From 21ed349c3946c80d10a5b1b3829ae0ab7a81b2fc Mon Sep 17 00:00:00 2001
From: Shaun Reed <shaunrd0@gmail.com>
Date: Mon, 28 Jun 2021 06:56:58 -0400
Subject: [PATCH] Fix array index offset bug in simple-graph traversal examples
 + Add comments to explain when to offset and when to use key values

---
 cpp/algorithms/graphs/simple/graph.cpp     |  4 +++-
 cpp/algorithms/graphs/simple/lib-graph.cpp | 27 +++++++++++-----------
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/cpp/algorithms/graphs/simple/graph.cpp b/cpp/algorithms/graphs/simple/graph.cpp
index c86b9de..e532a25 100644
--- a/cpp/algorithms/graphs/simple/graph.cpp
+++ b/cpp/algorithms/graphs/simple/graph.cpp
@@ -23,7 +23,7 @@ int main (const int argc, const char * argv[])
       {7, {3, 4, 6, 8}},
       {8, {4, 6}},
   };
-//  Graph bfsGraph(localNodes);
+  //  Graph bfsGraph(localNodes);
 
 
   std::cout << "\n\n##### Breadth First Search #####\n";
@@ -78,6 +78,8 @@ int main (const int argc, const char * argv[])
           {9, {}},
       }
   );
+
+
   // The graph traversed in this example is seen in MIT Intro to Algorithms
   // + Chapter 22, Figure 22.7 on Topological Sort
   // + Each node was replaced with a value from left-to-right, top-to-bottom
diff --git a/cpp/algorithms/graphs/simple/lib-graph.cpp b/cpp/algorithms/graphs/simple/lib-graph.cpp
index 175b95c..723ab6b 100644
--- a/cpp/algorithms/graphs/simple/lib-graph.cpp
+++ b/cpp/algorithms/graphs/simple/lib-graph.cpp
@@ -14,14 +14,13 @@
 void Graph::BFS(int startNode)
 {
   // Track the nodes we have discovered
-  std::vector<bool> discovered(nodes_.size());
-  for (bool node : discovered) node = false; // Initialize all nodes to false
+  std::vector<bool> discovered(nodes_.size(), false);
 
   // Create a queue to visit discovered nodes in FIFO order
   std::queue<int> visitQueue;
 
   // Visit the startNode
-  discovered[startNode] = true;
+  discovered[startNode - 1] = true;
   visitQueue.push(startNode);
 
   // Continue to visit nodes until there are none left in the graph
@@ -33,15 +32,17 @@ void Graph::BFS(int startNode)
     visitQueue.pop();
 
     // Check if we have already discovered all the adjacentNodes to thisNode
+    // + Do not offset this by 1, since we are using the key value for a map
     for (const auto &adjacent : nodes_[thisNode]) {
-      if (!discovered[adjacent]) {
+      if (!discovered[adjacent - 1]) {
         std::cout << "Found undiscovered adjacentNode: " << adjacent << "\n";
         // Mark the adjacent node as discovered
         // + If this were done out of the for loop we could discover nodes twice
         // + This would result in visiting the node twice, since it appears
         //    In the visitQueue twice
-        discovered[adjacent] = true;
+        discovered[adjacent - 1] = true;
         // Add the discovered node the the visitQueue
+        // + Since this value will later be used as a map key, dont offset by 1
         visitQueue.push(adjacent);
       }
     }
@@ -53,17 +54,16 @@ void Graph::BFS(int startNode)
 void Graph::DFS()
 {
   // Track the nodes we have discovered
-  std::vector<bool> discovered(nodes_.size());
-  for (auto node : discovered) node = false; // Initialize nodes to false
+  std::vector<bool> discovered(nodes_.size(), false);
 
   // Visit each node in the graph
   for (const auto &node : nodes_) {
     std::cout << "Visiting node " << node.first << std::endl;
     // If the node is undiscovered, visit it
-    if (!discovered[node.first]) {
+    if (!discovered[node.first - 1]) {
       std::cout << "Found undiscovered node: " << node.first << std::endl;
       // Mark the node as visited so we don't visit it twice
-      discovered[node.first] = true;
+      discovered[node.first - 1] = true;
       // Visiting the undiscovered node will check it's adjacent nodes
       DFSVisit(node.first, discovered);
     }
@@ -74,12 +74,13 @@ void Graph::DFS()
 void Graph::DFSVisit(int startNode, std::vector<bool> &discovered)
 {
   // Check the adjacent nodes of the startNode
+  // + Do not offset startNode by 1, since we use it as a key to a map
   for (auto &adjacent : nodes_[startNode]) {
     // If the adjacentNode is undiscovered, visit it
-    if (!discovered[adjacent]) {
+    if (!discovered[adjacent - 1]) {
       std::cout << "Found undiscovered adjacentNode: " << adjacent << std::endl;
       // Mark the node as visited so we don't visit it twice
-      discovered[adjacent] = true;
+      discovered[adjacent - 1] = true;
 
       // Visiting the undiscovered node will check it's adjacent nodes
       DFSVisit(adjacent, discovered);
@@ -93,8 +94,7 @@ std::vector<int> Graph::TopologicalSort()
   std::vector<int> topologicalOrder;
 
   // Track the nodes we have discovered
-  std::vector<bool> discovered(nodes_.size());
-  for (auto node : discovered) node = false; // Initialize nodes to false
+  std::vector<bool> discovered(nodes_.size(), false);
 
   // Visit each node in the graph
   for (const auto &node : nodes_) {
@@ -122,6 +122,7 @@ void Graph::TopologicalVisit(
   discovered[startNode - 1] = true;
 
   // Check the adjacent nodes of the startNode
+  // + Do not offset by 1, since startNode is used as a key to the map
   for (auto &adjacent : nodes_[startNode]) {
     // If the adjacentNode is undiscovered, visit it
     if (!discovered[adjacent - 1]) {