Cleanups for AbstractTree

bench/AbstractTreeBench.cpp

@@ -14,10 +14,10 @@ struct DataTree : chowdsp::AbstractTree<FakeData, DataTree>
     static Node& insertOneElement (FakeData&& element, Node& parent, AbstractTree& tree)
     {
         auto* new_node = tree.createLeafNode (std::move (element));
-        new_node->prev_sibling = parent.last_child;
-        if (parent.last_child != nullptr)
-            parent.last_child->next_sibling = new_node;
-        parent.last_child = new_node;
+        new_node->next_sibling = parent.first_child;
+        if (parent.first_child != nullptr)
+            parent.first_child->prev_sibling = new_node;
+        parent.first_child = new_node;
         return *new_node;
     }
 
@@ -27,25 +27,24 @@ struct DataTree : chowdsp::AbstractTree<FakeData, DataTree>
                                               std::string_view tag)
     {
         auto* new_sub_tree_node = tree.createTagNode (tag);
-        new_sub_tree_node->prev_sibling = parent.last_child;
-        if (parent.last_child != nullptr)
-            parent.last_child->next_sibling = new_sub_tree_node;
-        parent.last_child = new_sub_tree_node;
+        new_sub_tree_node->next_sibling = parent.first_child;
+        if (parent.first_child != nullptr)
+            parent.first_child->prev_sibling = new_sub_tree_node;
+        parent.first_child = new_sub_tree_node;
         return insertOneElement (std::move (element), *new_sub_tree_node, tree);
     }
 
     static FakeData& insertElementInternal (AbstractTree& self, FakeData&& element, Node& root)
     {
-        // since we know all the tags ahead of time, we only compare the first letter.
         for (auto* iter = root.first_child; iter != nullptr; iter = iter->next_sibling)
         {
-            if (iter->value.tag()[0] == positiveTag[0] && element[0] > 0)
+            if (iter->value.tag() == positiveTag && element[0] > 0)
                 return insertOneElement (std::move (element), *iter, self).value.leaf();
 
-            if (iter->value.tag()[0] == negativeTag[0] && element[0] < 0)
+            if (iter->value.tag() == negativeTag && element[0] < 0)
                 return insertOneElement (std::move (element), *iter, self).value.leaf();
 
-            if (iter->value.tag()[0] == zeroTag[0] && element[0] == 0)
+            if (iter->value.tag() == zeroTag && element[0] == 0)
                 return insertOneElement (std::move (element), *iter, self).value.leaf();
         }
 

modules/common/chowdsp_data_structures/Structures/chowdsp_AbstractTree.cpp

@@ -43,33 +43,33 @@ void AbstractTree<ElementType, DerivedType>::insertNodeSorted (Node& parent, Nod
     if (parent.first_child == nullptr)
     {
         parent.first_child = new_node;
-        parent.last_child = new_node;
         return;
     }
 
     // insert into the parents children, sorted
-    for (auto* iter = parent.first_child; iter != nullptr; iter = iter->next_sibling)
+    Node* prev {};
+    for (Node* iter = parent.first_child; iter != nullptr; iter = iter->next_sibling)
     {
         if (comparator (*new_node, *iter))
         {
             new_node->next_sibling = iter;
             new_node->prev_sibling = iter->prev_sibling;
             iter->prev_sibling = new_node;
 
-            if (auto* prev_sibling = new_node->prev_sibling; prev_sibling != nullptr)
-                prev_sibling->next_sibling = new_node;
+            if (prev != nullptr)
+                prev->next_sibling = new_node;
 
             if (iter == parent.first_child)
                 parent.first_child = new_node;
 
             return;
         }
+        prev = iter;
     }
 
     // insert at the end of the parents children
-    parent.last_child->next_sibling = new_node;
-    new_node->prev_sibling = parent.last_child;
-    parent.last_child = new_node;
+    prev->next_sibling = new_node;
+    new_node->prev_sibling = prev;
 }
 
 template <typename ElementType, typename DerivedType>
@@ -99,18 +99,15 @@ void AbstractTree<ElementType, DerivedType>::removeNode (Node& node)
     if (node.next_sibling != nullptr)
         node.next_sibling->prev_sibling = node.prev_sibling;
 
-    if (node.parent->first_child == node.parent->last_child)
+    if (node.prev_sibling == nullptr && node.next_sibling == nullptr)
     {
         node.parent->first_child = nullptr;
-        node.parent->last_child = nullptr;
         removeNode (*node.parent);
     }
     else
     {
         if (node.parent->first_child == &node)
             node.parent->first_child = node.next_sibling;
-        if (node.parent->last_child == &node)
-            node.parent->last_child = node.prev_sibling;
     }
 
     node.value.destroy();
@@ -138,16 +135,11 @@ void AbstractTree<ElementType, DerivedType>::removeElements (const Callable& ele
     // need to change.
     for (auto* node = &root_node; node != nullptr;)
     {
+        auto* next_node = node->next_linear;
         if (node->value.has_value() && elementsToRemove (node->value.leaf()))
-        {
-            auto* next_node = node->next_linear;
             removeNode (*node);
-            node = next_node;
-        }
-        else
-        {
-            node = node->next_linear;
-        }
+
+        node = next_node;
     }
 }
 

modules/common/chowdsp_data_structures/Structures/chowdsp_AbstractTree.h

@@ -81,7 +81,6 @@ class AbstractTree
 
         Node* parent {}; // slot for parent in hierarchy
         Node* first_child {}; // slot for first child in hierarchy
-        Node* last_child {}; // slot for last child in hierarchy
         Node* next_sibling {}; // slot for next sibling in hierarchy
         Node* prev_sibling {}; // slot for previous sibling in hierarchy
         Node* next_linear {}; // slot for linked list through all nodes
@@ -107,7 +106,10 @@ class AbstractTree
     /** Removes a node. */
     void removeNode (Node& node);
 
-    /** Removes a element by value. */
+    /**
+     * Removes a element by value.
+     * If the tree contains multiple identical elements, only the first will be removed.
+     */
     void removeElement (const ElementType& element);
 
     /**

modules/plugin/chowdsp_presets_v2/Frontend/chowdsp_PresetsNextPreviousInterface.cpp

@@ -16,6 +16,17 @@ static const PresetTree::Node* findNodeForPreset (const PresetTree::Node& root,
     return nullptr;
 }
 
+static const PresetTree::Node* getNextOrPreviousChildPresetNode (const PresetTree::Node* node, bool forward)
+{
+    if (forward)
+        return node->first_child;
+
+    node = node->first_child;
+    while (node->next_sibling != nullptr)
+        node = node->next_sibling;
+    return node;
+}
+
 static const PresetTree::Node* getNextOrPreviousPresetNode (const PresetTree::Node* node, bool forward)
 {
     if (node == nullptr)
@@ -31,7 +42,7 @@ static const PresetTree::Node* getNextOrPreviousPresetNode (const PresetTree::No
     if (nextParent->value.has_value())
         return nextParent;
 
-    return forward ? nextParent->first_child : nextParent->last_child;
+    return getNextOrPreviousChildPresetNode (nextParent, forward);
 }
 
 bool NextPrevious::navigateThroughPresets (bool forward)
@@ -55,7 +66,7 @@ bool NextPrevious::navigateThroughPresets (bool forward)
     }
 
     while (! nextPresetNode->value.has_value())
-        nextPresetNode = forward ? nextPresetNode->first_child : nextPresetNode->last_child;
+        nextPresetNode = getNextOrPreviousChildPresetNode (nextPresetNode, forward);
 
     presetManager.loadPreset (nextPresetNode->value.leaf());
     return true;

tests/common_tests/chowdsp_data_structures_test/AbstractTreeTest.cpp

@@ -118,7 +118,7 @@ TEST_CASE ("Abstract Tree Test", "[common][data-structures]")
         tree.removeElement ("donuts");
         REQUIRE (tree.size() == 3);
 
-        const auto* b_node = tree.getRootNode().last_child;
+        const auto* b_node = tree.getRootNode().first_child->next_sibling;
         REQUIRE (b_node->value.tag() == "b");
     }
 
@@ -128,7 +128,8 @@ TEST_CASE ("Abstract Tree Test", "[common][data-structures]")
                              { return el.find ('t') != std::string::npos; });
         REQUIRE (tree.size() == 2);
 
-        REQUIRE (tree.getRootNode().first_child == tree.getRootNode().last_child);
+        REQUIRE (tree.getRootNode().first_child->next_sibling == nullptr);
+        REQUIRE (tree.getRootNode().first_child->prev_sibling == nullptr);
         REQUIRE (tree.getRootNode().first_child->value.tag() == "a");
     }
 

tests/plugin_tests/chowdsp_presets_v2_test/PresetTreeTest.cpp

@@ -89,7 +89,7 @@ TEST_CASE ("Preset Tree Test", "[plugin][presets]")
         REQUIRE (drums_node->first_child->next_sibling->next_sibling == nullptr);
 
         const auto* loose_preset = drums_node->next_sibling;
-        REQUIRE (loose_preset == root.last_child);
+        REQUIRE (loose_preset->next_sibling == nullptr);
         REQUIRE (loose_preset->value.leaf().getName() == "Preset1");
     }
 
@@ -126,7 +126,7 @@ TEST_CASE ("Preset Tree Test", "[plugin][presets]")
         REQUIRE (jatin_drums_node->first_child->next_sibling == nullptr);
 
         const auto* jatin_loose_preset = jatin_drums_node->next_sibling;
-        REQUIRE (jatin_loose_preset == jatin_node->last_child);
+        REQUIRE (jatin_loose_preset->next_sibling == nullptr);
         REQUIRE (jatin_loose_preset->value.leaf().getName() == "Blah");
 
         const auto* steve_node = jatin_node->next_sibling;