fixed a bug due to using pointers instead of vector indices

Map.hpp

@@ -4,6 +4,7 @@
 #include <cassert>
 #include <cstddef>
 #include <initializer_list>
+#include <optional>
 #include <stdexcept>
 #include <utility>
 #include <vector>
@@ -38,11 +39,11 @@ template <typename Key_T, typename Mapped_T> struct BookKeeping {
   // Ptr self;
   Color color;
   // nullptr indicates empty
-  Self *parent;
-  Self *left;
-  Self *right;
-  Self *prev;
-  Self *next;
+  std::optional<std::size_t> parent;
+  std::optional<std::size_t> left;
+  std::optional<std::size_t> right;
+  std::optional<std::size_t> prev;
+  std::optional<std::size_t> next;
   BookKeeping(Map<Key_T, Mapped_T> &container) : container{container} {}
   BookKeeping(BookKeeping const &rhs)
       : container{rhs.container}, value{rhs.value}, // self{rhs.self},
@@ -65,7 +66,11 @@ template <typename Key_T, typename Mapped_T> struct BookKeeping {
     return *this;
   }
   // reference to a pointer because the alternatives were worse
-  inline Self *&child(Direction dir) {
+  inline Self *child(Direction dir) {
+    auto ret = c_select(dir);
+    return ret.has_value() ? &container.nodes[left.value()] : nullptr;
+  }
+  inline std::optional<std::size_t> c_select(Direction dir) {
     switch (dir) {
     case Direction::Left:
       return left;
@@ -77,6 +82,64 @@ template <typename Key_T, typename Mapped_T> struct BookKeeping {
       assert(false);
     }
   }
+  inline void c_trans(Direction dir, Self *v) {
+    switch (dir) {
+    case Direction::Left:
+      this->set_l(v);
+      break;
+    case Direction::Right:
+      this->set_r(v);
+      break;
+    default:
+      assert(false);
+    }
+  }
+
+  inline Self *n() {
+    return next.has_value() ? &container.nodes[next.value()] : nullptr;
+  }
+  inline void set_n(Self *ptr) {
+    this->next = ptr == nullptr
+                     ? std::nullopt
+                     : std::optional<std::size_t>{static_cast<std::size_t>(
+                           ptr - &this->container.nodes[0])};
+  }
+  inline Self *p() {
+    return prev.has_value() ? &container.nodes[prev.value()] : nullptr;
+  }
+  inline void set_p(Self *ptr) {
+    this->prev = ptr == nullptr
+                     ? std::nullopt
+                     : std::optional<std::size_t>{static_cast<std::size_t>(
+                           ptr - &this->container.nodes[0])};
+  }
+  inline Self *r() {
+    return right.has_value() ? &container.nodes[right.value()] : nullptr;
+  }
+  inline void set_r(Self *ptr) {
+    this->right = ptr == nullptr
+                      ? std::nullopt
+                      : std::optional<std::size_t>{static_cast<std::size_t>(
+                            ptr - &this->container.nodes[0])};
+  }
+  inline Self *l() {
+    return left.has_value() ? &container.nodes[left.value()] : nullptr;
+  }
+  inline void set_l(Self *ptr) {
+    this->left = ptr == nullptr
+                     ? std::nullopt
+                     : std::optional<std::size_t>{static_cast<std::size_t>(
+                           ptr - &this->container.nodes[0])};
+  }
+  inline Self *par() {
+    return parent.has_value() ? &container.nodes[parent.value()] : nullptr;
+  }
+  inline void set_par(Self *ptr) {
+    this->parent = ptr == nullptr
+                       ? std::nullopt
+                       : std::optional<std::size_t>{static_cast<std::size_t>(
+                             ptr - &this->container.nodes[0])};
+  }
   // this is root/P for this method
   // copying from wikipedia RotateDirRoot with translation into my own idioms
   // https://en.wikipedia.org/wiki/Red%E2%80%93black_tree#Operations
@@ -84,31 +147,30 @@ template <typename Key_T, typename Mapped_T> struct BookKeeping {
     // wikipedia version uses alphabet soup, might fix later
     Self *P = this;
     auto &T = container;
-    Self *G = P->parent;
+    Self *G = P->par();
     Self *S = P->child(!dir);
     Self *C;
 
     // this method shouldn't be called in cases where this assert will trip
     assert(S != nullptr);
 
-    //
     C = S->child(dir);
-    P->child(!dir) = C;
+    P->c_trans(!dir, C);
 
     if (C != nullptr) {
-      C->parent = P;
+      C->set_par(P);
     }
-    S->child(dir) = P;
-    P->parent = S;
-    S->parent = G;
+    S->c_trans(dir, P);
+    P->set_par(S);
+    S->set_par(G);
     if (G != nullptr) {
-      if (P == G->right) {
-        G->right = S;
+      if (P == G->r()) {
+        G->set_r(S);
       } else {
-        G->left = S;
+        G->set_l(S);
       }
     } else {
-      T.root = S;
+      T.root = S - &T.nodes[0];
     }
   }
 };
@@ -134,22 +196,25 @@ public:
     friend Node;
 
   private:
-    // pointer needed so we can replace as needed
-    Node *ref;
-    Node *escape;
-    Iterator(Node *ref, Node *escape = nullptr) : ref{ref}, escape{escape} {}
+    using Ref_T = std::optional<std::size_t>;
+    Map<Key_T, Mapped_T> &parent;
+    Ref_T ref;
+    Ref_T escape;
+    Iterator(Map<Key_T, Mapped_T> &parent, Ref_T ref,
+             Ref_T escape = std::nullopt)
+        : parent{parent}, ref{ref}, escape{escape} {}
 
   public:
     Iterator() = delete;
     Iterator &operator++() {
-      if (ref == nullptr) {
+      if (ref == std::nullopt) {
         ref = escape;
         return *this;
       }
-      if (ref->next == nullptr) {
+      if (parent.nodes[ref.value()].next == std::nullopt) {
         escape = ref;
       }
-      ref = ref->next;
+      ref = parent.nodes[ref.value()].next;
       return *this;
     }
     Iterator operator++(int) {
@@ -158,14 +223,14 @@ public:
       return tmp;
     }
     Iterator &operator--() {
-      if (ref == nullptr) {
+      if (ref == std::nullopt) {
         ref = escape;
         return *this;
       }
-      if (ref->prev == nullptr) {
+      if (parent.nodes[ref.value()].prev == std::nullopt) {
         escape = ref;
       }
-      ref = ref->prev;
+      ref = parent.nodes[ref.value()].prev;
       return *this;
     }
     Iterator operator--(int) {
@@ -173,8 +238,10 @@ public:
       --(*this);
       return tmp;
     }
-    ValueType &operator*() const { return this->ref->value; }
-    ValueType *operator->() const { return &this->ref->value; }
+    ValueType &operator*() const {
+      return this->parent.nodes[this->ref.value()].value;
+    }
+    ValueType *operator->() const { return &this->operator*(); }
     friend bool operator==(Iterator const &lhs, Iterator const &rhs) {
       return lhs.ref == rhs.ref;
     }
@@ -277,58 +344,63 @@ public:
   };
 
 private:
-  Node *root;
-  Node *min;
-  Node *max;
+  std::optional<std::size_t> root;
+  std::optional<std::size_t> min;
+  std::optional<std::size_t> max;
   std::vector<Node> nodes;
+  std::size_t size_diff;
 
 public:
-  Map() : root{nullptr}, min{nullptr}, max{nullptr}, nodes{} {}
+  Map()
+      : root{std::nullopt}, min{std::nullopt}, max{std::nullopt}, nodes{},
+        size_diff{0} {}
   Map(const Map &rhs)
-      : root{rhs.root}, min{nullptr}, max{nullptr}, nodes{rhs.nodes} {}
+      : root{rhs.root}, min{rhs.min}, max{rhs.max}, nodes{rhs.nodes},
+        size_diff{0} {}
   Map &operator=(const Map &rhs) {
-    // TODO: fix this so all the pointers in all the bookkeeping nodes are
-    // updated in addition to root, min and max
-    assert(false);
     this->root = rhs.root;
     this->min = rhs.min;
     this->max = rhs.max;
     this->nodes = rhs.nodes;
   }
-  Map(std::initializer_list<ValueType> elems) : root{nullptr}, nodes{} {
+  Map(std::initializer_list<ValueType> elems)
+      : root{std::nullopt}, min{std::nullopt}, max{std::nullopt}, nodes{} {
     this->insert(elems.begin(), elems.end());
   }
 
   ~Map() {}
 
-  size_t size() const {
-    root = nullptr;
-    return this->nodes.size();
-  }
+  size_t size() const { return this->nodes.size() - this->size_diff; }
   bool empty() const { return this->size() == 0; }
-  Iterator begin() { return Iterator{min}; }
-  Iterator end() { return Iterator{nullptr, max}; }
-  ConstIterator begin() const { return ConstIterator{this->begin()}; }
-  ConstIterator end() const { return ConstIterator{this->end()}; }
+
+  Iterator begin() { return Iterator{*this, min}; }
+  Iterator end() { return Iterator{*this, std::nullopt, max}; }
+  ConstIterator begin() const { return ConstIterator{*this, this->begin()}; }
+  ConstIterator end() const { return ConstIterator{*this, this->end()}; }
   ConstIterator cbegin() const { return this->begin(); }
   ConstIterator cend() const { return this->end(); }
-  ReverseIterator rbegin() { return ReverseIterator{Iterator{this->max}}; }
-  ReverseIterator rend() { return ReverseIterator{Iterator{nullptr, min}}; }
+  ReverseIterator rbegin() {
+    return ReverseIterator{Iterator{*this, this->max}};
+  }
+  ReverseIterator rend() {
+    return ReverseIterator{Iterator{*this, nullptr, min}};
+  }
   Iterator find(const Key_T &key) {
     // we need a locate slot function for insert regardless so might as well use
     // it here
     auto [parent, dir] = this->locate_slot(key);
-    if (parent == nullptr) {
-      if (this->root != nullptr && this->root->value.first == key) {
-        return Iterator{root};
+    if (!parent.has_value()) {
+      if (this->root.has_value() &&
+          this->nodes[this->root.value()].value.first == key) {
+        return Iterator{*this, root};
       } else {
         return this->end();
       }
     }
-    if (parent->child(dir) == nullptr) {
+    if (!this->nodes[parent.value()].c_select(dir).has_value()) {
       return this->end();
     }
-    return Iterator{parent->child(dir)};
+    return Iterator{*this, this->nodes[parent.value()].c_select(dir)};
   }
   // implicit cast to ConstIterator from Iterator
   ConstIterator find(const Key_T &key) const { return this->find(key); }
@@ -355,6 +427,14 @@ public:
   }
 
 private:
+  void handle_root_rotation(std::optional<std::size_t> g,
+                            std::optional<std::size_t> p,
+                            std::optional<std::size_t> i, Direction dir) {
+    handle_root_rotation(g.has_value() ? &this->nodes[g.value()] : nullptr,
+                         p.has_value() ? &this->nodes[p.value()] : nullptr,
+                         i.has_value() ? &this->nodes[i.value()] : nullptr,
+                         dir);
+  }
   void handle_root_rotation(Node *grandparent, Node *parent, Node *inserting,
                             Direction dir) {
     // making inner grandchild into outer grandchild
@@ -364,95 +444,101 @@ private:
       parent = grandparent->child(dir);
     }
     // RotateDirRoot(T,G,1-dir);
-    Node *gr_grandparent = grandparent->parent;
+    Node *gr_grandparent = grandparent->par();
     Node *sibling = grandparent->child(!dir);
     assert(sibling != nullptr);
     Node *child = sibling->child(dir);
-    grandparent->child(!dir) = child;
-    sibling->child(dir) = grandparent;
-    grandparent->parent = sibling;
-    sibling->parent = gr_grandparent;
+    grandparent->c_trans(!dir, child);
+    sibling->c_trans(dir, grandparent);
+    grandparent->set_par(sibling);
+    sibling->set_par(gr_grandparent);
     if (gr_grandparent != nullptr) {
       Direction grandparent_direction;
-      if (gr_grandparent->left == grandparent) {
+      if (gr_grandparent->l() == grandparent) {
         grandparent_direction = Direction::Left;
       } else {
         grandparent_direction = Direction::Right;
       }
-      gr_grandparent->child(grandparent_direction) = sibling;
+      gr_grandparent->c_trans(grandparent_direction, sibling);
     } else {
-      this->root = sibling;
+      this->root = sibling - &this->nodes[0];
     }
 
     parent->color = Color::Black;
     grandparent->color = Color::Red;
   }
+  using Ref_T = std::optional<std::size_t>;
   // heavily referencing the wikipedia implementation for this
   // https://en.wikipedia.org/wiki/Red%E2%80%93black_tree#Insertion
-  void insert_helper(Node *to_insert, Node *parent, Direction dir) {
+  void insert_helper(Ref_T to_insert, Ref_T parent, Direction dir) {
     // initialize the element we're inserting
-    to_insert->color = Color::Red;
-    to_insert->left = nullptr;
-    to_insert->right = nullptr;
-    to_insert->parent = parent;
-    switch (dir) {
-    case Direction::Left:
-      to_insert->next = parent;
-      to_insert->prev = parent->prev;
-      parent->prev = to_insert;
-      parent->left = to_insert;
-      break;
-    case Direction::Right:
-      to_insert->prev = parent;
-      to_insert->next = parent->next;
-      parent->next = to_insert;
-      parent->right = to_insert;
-      break;
-    }
+    this->nodes[to_insert.value()].color = Color::Red;
+    this->nodes[to_insert.value()].left = std::nullopt;
+    this->nodes[to_insert.value()].right = std::nullopt;
+    this->nodes[to_insert.value()].next = std::nullopt;
+    this->nodes[to_insert.value()].prev = std::nullopt;
+    this->nodes[to_insert.value()].parent = parent;
 
     // if this is the first element to be inserted it's root
-    if (to_insert->parent == nullptr) {
+    if (!this->nodes[to_insert.value()].parent.has_value()) {
       this->root = to_insert;
+      this->nodes[to_insert.value()].color = Color::Black;
       return;
     }
 
     switch (dir) {
     case Direction::Left:
-      parent->left = to_insert;
+      this->nodes[to_insert.value()].next = parent;
+      this->nodes[to_insert.value()].prev = this->nodes[parent.value()].prev;
+      this->nodes[parent.value()].prev = to_insert;
+      this->nodes[parent.value()].left = to_insert;
       break;
     case Direction::Right:
-      parent->right = to_insert;
+      this->nodes[to_insert.value()].prev = parent;
+      this->nodes[to_insert.value()].next = this->nodes[parent.value()].next;
+      this->nodes[parent.value()].next = to_insert;
+      this->nodes[parent.value()].right = to_insert;
+      break;
+    }
+
+    switch (dir) {
+    case Direction::Left:
+      this->nodes[parent.value()].left = to_insert;
+      break;
+    case Direction::Right:
+      this->nodes[parent.value()].right = to_insert;
       break;
     }
 
     do {
       // don't need to keep track of these in between loops they get
       // recalculated
-      Node *grandparent;
-      Node *uncle;
-      if (parent->color == Color::Black) {
+      std::optional<std::size_t> grandparent;
+      std::optional<std::size_t> uncle;
+      if (this->nodes[parent.value()].color == Color::Black) {
         // black parent means invariants definitely hold
         return;
       }
 
-      grandparent = parent->parent;
+      grandparent = this->nodes[parent.value()].parent;
 
-      if (grandparent == nullptr) {
+      if (!grandparent.has_value()) {
         // parent is root, just need to recolor it to black
-        parent->color = Color::Black;
+        this->nodes[parent.value()].color = Color::Black;
         return;
       }
 
       Direction parent_direction;
-      if (grandparent->left == parent) {
+      if (this->nodes[grandparent.value()].left == parent) {
         parent_direction = Direction::Left;
-        uncle = grandparent->right;
+        uncle = this->nodes[grandparent.value()].right;
       } else {
         parent_direction = Direction::Right;
-        uncle = grandparent->left;
+        uncle = this->nodes[grandparent.value()].left;
       }
 
-      if (uncle == nullptr || uncle->color == Color::Black) {
+      if (!uncle.has_value() ||
+          this->nodes[uncle.value()].color == Color::Black) {
         // case 5 and 6
         this->handle_root_rotation(grandparent, parent, to_insert,
                                    parent_direction);
@@ -461,29 +547,32 @@ private:
 
       // now we know parent and uncle are both red so red-black coloring can be
       // pushed down from grandparent
-      parent->color = Color::Black;
-      uncle->color = Color::Black;
-      grandparent->color = Color::Red;
+      this->nodes[parent.value()].color = Color::Black;
+      this->nodes[uncle.value()].color = Color::Black;
+      this->nodes[grandparent.value()].color = Color::Red;
 
       to_insert = grandparent;
-      parent = to_insert->parent;
-    } while (parent != nullptr);
+      parent = this->nodes[to_insert.value()].parent;
+    } while (parent.has_value());
 
     // case 3: current node is red root so we're done
   }
   // returns nullptr iff map is empty
-  std::pair<Node *, Direction> locate_slot(const Key_T &key) {
-    Node *current = this->root;
-    Node *parent = nullptr;
+  std::pair<std::optional<std::size_t>, Direction>
+  locate_slot(const Key_T &key) {
+    using Ref_T = std::optional<std::size_t>;
+    Ref_T current = this->root;
+    Ref_T parent = std::nullopt;
     Direction dir;
-    while (current != nullptr && current->value.first != key) {
+    while (current.has_value() &&
+           this->nodes[current.value()].value.first != key) {
       parent = current;
-      if (key < current->value.first) {
+      if (key < this->nodes[current.value()].value.first) {
         dir = Direction::Left;
-        current = current->left;
+        current = this->nodes[current.value()].left;
       } else {
         dir = Direction::Right;
-        current = current->right;
+        current = this->nodes[current.value()].right;
       }
     }
     return std::make_pair(parent, dir);
@@ -496,26 +585,30 @@ public:
   // an iterator pointing to the element with the same key, and false.
   std::pair<Iterator, bool> insert(const ValueType &val) {
     auto [parent, dir] = locate_slot(val.first);
-    bool ret = parent == nullptr || parent->child(dir) == nullptr;
+    bool ret = !parent.has_value() ||
+               !this->nodes[parent.value()].c_select(dir).has_value();
     if (!ret) {
-      return std::make_pair(Iterator{parent->child(dir)}, ret);
+      return std::make_pair(
+          Iterator{*this, this->nodes[parent.value()].c_select(dir)}, ret);
     }
     Node to_insert{*this};
     to_insert.value = val;
     this->nodes.push_back(std::move(to_insert));
     // this->nodes.back().self = (--this->nodes.end());
-    insert_helper(&nodes.back(), parent, dir);
+    insert_helper(nodes.size() - 1, parent, dir);
 
-    if (min == nullptr || val.first < min->value.first) {
-      min = &nodes.back();
-      nodes.back().prev = nullptr;
+    if (min == std::nullopt ||
+        val.first < this->nodes[min.value()].value.first) {
+      min = nodes.size() - 1;
+      nodes.back().prev = std::nullopt;
     }
-    if (max == nullptr || val.first > max->value.first) {
-      max = &nodes.back();
-      nodes.back().next = nullptr;
+    if (max == std::nullopt ||
+        val.first > this->nodes[max.value()].value.first) {
+      max = nodes.size() - 1;
+      nodes.back().next = std::nullopt;
     }
 
-    return std::make_pair(Iterator(&nodes.back()), ret);
+    return std::make_pair(Iterator(*this, nodes.size() - 1), ret);
   }
   template <typename IT_T> void insert(IT_T range_beg, IT_T range_end) {
     std::for_each(range_beg, range_end,
@@ -542,22 +635,22 @@ private:
   // https://en.wikipedia.org/wiki/Red%E2%80%93black_tree#Removal_of_a_black_non-root_leaf
   void complex_erase(Iterator pos) {
 
-    Node *to_delete = pos.ref;
-    Node *parent = to_delete->parent;
+    Node *to_delete = &this->nodes[pos.ref.value()];
+    Node *parent = to_delete->par();
     assert(parent != nullptr);
 
     Direction dir =
-        parent->right == to_delete ? Direction::Right : Direction::Left;
+        parent->r() == to_delete ? Direction::Right : Direction::Left;
 
     Node *sibling;
     ;
     Node *close_nephew;
     Node *distant_nephew;
 
-    parent->child(dir) = nullptr;
+    parent->c_trans(dir, nullptr);
 
     do {
-      dir = parent->right == to_delete ? Direction::Right : Direction::Left;
+      dir = parent->r() == to_delete ? Direction::Right : Direction::Left;
 
       sibling = parent->child(!dir);
       distant_nephew = sibling->child(!dir);
@@ -605,56 +698,67 @@ private:
       // case 2
       sibling->color = Color::Red;
       to_delete = parent;
-      parent = to_delete->parent;
+      parent = to_delete->par();
     } while (parent != nullptr);
   }
 
 public:
   // TODO: check that the way of reconnecting next and prev works
   void erase(Iterator pos) {
+    this->size_diff++;
     // simple cases
-    Node *ref = pos.ref;
-    if (ref == this->min) {
+    Node *ref = &this->nodes[pos.ref.value()];
+    if (ref ==
+        (this->min.has_value() ? &this->nodes[this->min.value()] : nullptr)) {
       this->min = ref->next;
     }
-    if (ref == this->max) {
+    if (ref ==
+        (this->max.has_value() ? &this->nodes[this->max.value()] : nullptr)) {
       this->max = ref->prev;
     }
     // 2 children
-    if (ref->left != nullptr && ref->right != nullptr) {
-      Node *next = ref->next;
-      Node *prev = ref->prev;
-      *ref = *next;
-      prev->next = next;
-      next->prev = prev;
-      this->erase(Iterator{next});
+    if (ref->l() != nullptr && ref->r() != nullptr) {
+      Ref_T next = ref->next;
+      Ref_T prev = ref->prev;
+      *ref = this->nodes[next.value()];
+      this->nodes[prev.value()].next = next;
+      this->nodes[next.value()].prev = prev;
+      this->erase(Iterator{*this, next});
     }
     // single child which is left
-    else if (ref->left != nullptr && ref->right == nullptr) {
-      Node *next = ref->next;
-      Node *prev = ref->prev;
-      *ref = *ref->left;
-      prev->next = next;
-      next->prev = prev;
+    else if (ref->l() != nullptr && ref->r() == nullptr) {
+      Ref_T next = ref->next;
+      Ref_T prev = ref->prev;
+      *ref = *ref->l();
+      this->nodes[prev.value()].next = next;
+      this->nodes[next.value()].prev = prev;
     }
     // single child which is right
-    else if (ref->left == nullptr && ref->right != nullptr) {
-      Node *next = ref->next;
-      Node *prev = ref->prev;
-      *ref = *ref->right;
-      prev->next = next;
-      next->prev = prev;
+    else if (ref->l() == nullptr && ref->r() != nullptr) {
+      Ref_T next = ref->next;
+      Ref_T prev = ref->prev;
+      *ref = *ref->r();
+      if (prev.has_value()) {
+        this->nodes[prev.value()].next = next;
+      }
+      if (next.has_value()) {
+        this->nodes[next.value()].prev = prev;
+      }
     }
     // no children and root
-    else if (ref->left == nullptr && ref->right == nullptr) {
-      this->root = nullptr;
+    else if (ref->l() == nullptr && ref->r() == nullptr) {
+      this->root = std::nullopt;
     }
     // no children and red
-    else if (ref->left == nullptr && ref->right == nullptr) {
-      Node *next = ref->next;
-      Node *prev = ref->prev;
-      prev->next = next;
-      next->prev = prev;
+    else if (ref->l() == nullptr && ref->r() == nullptr) {
+      Node *next =
+          ref->next.has_value() ? &this->nodes[ref->next.value()] : nullptr;
+      Node *prev =
+          ref->prev.has_value() ? &this->nodes[ref->prev.value()] : nullptr;
+      prev->next = next != nullptr ? std::optional{next - &this->nodes[0]}
+                                   : std::nullopt;
+      next->prev = prev != nullptr ? std::optional{prev - &this->nodes[0]}
+                                   : std::nullopt;
     }
     // complicated case of black node with no kids
     else {
@@ -663,7 +767,7 @@ public:
   }
   void erase(const Key_T &key) { this->erase(this->find(key)); }
   void clear() {
-    this->root = nullptr;
+    this->root = std::nullopt;
     this->nodes.clear();
   }
   friend bool operator==(const Map &lhs, const Map &rhs) {