commenting is done

Map.hpp

@@ -214,6 +214,10 @@ template <typename Key_T, typename Mapped_T> class Map {
       assert(false);
     }
 
+    // erase_child fully deletes the child node, this includes
+    // having the child's memory be freed so any data the child holds
+    // which is needed for future calculations must be gathered before
+    // it is erased
     void erase_child(Node *n) { this->erase_child(this->which_child(n)); }
     void erase_child(Direction dir) {
       // if the child being erased is the min or the max we need to update
@@ -754,23 +758,22 @@ private:
 
   // the hard case of removing a black leaf node in a red black tree
   // All other cases are handled in core_erase
-  void hard_erase(Node *n) {
-    assert(n->parent);
-    Node *parent = n->parent;
-    Direction dir = parent->which_child(n);
-    parent->erase_child(n);
+  void hard_erase(Node *target) {
+    assert(target->parent);
 
-    // case 1 should be handled first for all but the first loop
-    // so it's skipped in the first loop
-    goto skip;
+    // parent and direction need to be gathered here due to imminent
+    // deletion of the target from memory
+    Node *parent = target->parent;
+    Direction dir = parent->which_child(target);
+
+    // Note: target is now deallocated and should not be used again
+    // until we reassign it in the loop
+    parent->erase_child(target);
+
+    // refer to
+    // https://en.wikipedia.org/wiki/Red%E2%80%93black_tree#Notes_to_the_delete_diagrams
+    // for more details on what all of this is ultimately doing
     while (true) {
-      parent = n->parent;
-      if (parent == nullptr) {
-        // we're at root we're done (case 1)
-        return;
-      }
-      dir = parent->which_child(n);
-    skip:
       Color par_color = parent->color;
 
       Node *sibling = parent->child(!dir);
@@ -782,15 +785,11 @@ private:
       Color close_color = close ? close->color : Color::Black;
       Color distant_color = distant ? distant->color : Color::Black;
 
-      // Macro is used for simplicity and avoiding repetition
+      // Macro is used for avoiding repetition
 #define redcheck(v) if ((v) == Color::Red)
 
-      // TODO: continue comment writing here
-      // it kinda sucks but I think that goto is genuinely the best solution
-      // here, making methods for cases 4,5 and 6 is a lot of unneeded
-      // bookkeeping
       redcheck(sibling_color) {
-        // case 3
+        // Case 3
         if (parent->parent) {
           parent->rotate(dir);
         } else {
@@ -801,6 +800,14 @@ private:
         sibling = close;
 
         distant = sibling->child(!dir);
+        // Checking after conditions
+        // GOTO is used due to heavy sharing of state between
+        // cases which would be annoying to pass as procedure
+        // argument(s) especially for handling the various places
+        // cases 4, 5 and 6 show up at
+        // In theory this could've been better handled as an
+        // explicit FSM object but it was easier to do this via
+        // GOTO in the moment
         if (distant != nullptr && distant->color == Color::Red) {
           goto case_6;
         }
@@ -811,7 +818,6 @@ private:
         goto case_4;
       }
       else redcheck(close_color) {
-      // case 5
       case_5:
         assert(sibling);
         assert(close);
@@ -823,7 +829,6 @@ private:
         goto case_6;
       }
       else redcheck(distant_color) {
-        // case 6
       case_6:
         assert(parent);
         assert(sibling);
@@ -840,7 +845,6 @@ private:
         return;
       }
       else redcheck(par_color) {
-      // case 4
       case_4:
         assert(sibling);
         sibling->color = Color::Red;
@@ -848,12 +852,22 @@ private:
         return;
       }
       else {
-        // case 2
+        // Case 2: we need to keep cranking through
+        // the motions backtracking up the tree so we
+        // reassign target appropriately after recoloring
         assert(sibling);
         sibling->color = Color::Red;
-        n = parent;
-        continue;
+        target = parent;
       }
+      // Case 1: After backtracking up the tree we may reach the root
+      // node in which case we've finished resetting our invariants
+      // if we aren't at the root node this code will update parent
+      // and dir to appropriate values
+      parent = target->parent;
+      if (parent == nullptr) {
+        return;
+      }
+      dir = parent->which_child(target);
     }
   }