From 2290e1edaf606dfd90f95493efa4e2940022706e Mon Sep 17 00:00:00 2001
From: Rainer Kartmann <rainer.kartmann@kit.edu>
Date: Thu, 5 Aug 2021 13:13:38 +0200
Subject: [PATCH] Fix deprecations by making TreeWidgetBuilder more flexible

---
 .../libraries/armem_gui/TreeWidgetBuilder.h   | 95 ++++++++++++++-----
 .../instance/MemoryIDTreeWidgetItem.cpp       |  6 +-
 .../tree_builders/DataTreeBuilder.cpp         |  4 +-
 .../tree_builders/DataTreeBuilderBase.h       |  4 +-
 .../tree_builders/TypedDataTreeBuilder.cpp    | 10 +-
 .../libraries/armem_gui/memory/TreeWidget.cpp | 54 +++++++----
 .../libraries/armem_gui/memory/TreeWidget.h   | 11 +--
 7 files changed, 123 insertions(+), 61 deletions(-)

diff --git a/source/RobotAPI/libraries/armem_gui/TreeWidgetBuilder.h b/source/RobotAPI/libraries/armem_gui/TreeWidgetBuilder.h
index 37de7699c..f3ca45cf1 100644
--- a/source/RobotAPI/libraries/armem_gui/TreeWidgetBuilder.h
+++ b/source/RobotAPI/libraries/armem_gui/TreeWidgetBuilder.h
@@ -16,10 +16,10 @@ namespace armarx
      * A class to efficiently build and maintain sorted items of `QTreeWidget`
      * or `QTreeWidgetItem` based on a sorted container matching the intended structure.
      */
-    template <class ContainerT>
+    template <class _ElementT>
     struct TreeWidgetBuilder
     {
-        using ElementT = typename ContainerT::value_type;
+        using ElementT = _ElementT;
 
         /// Return < 0 if `element < item`, 0 if `element == item`, and > 0 if `element > item`.
         using CompareFn = std::function<int(const ElementT& element, QTreeWidgetItem* item)>;
@@ -30,14 +30,15 @@ namespace armarx
 
         TreeWidgetBuilder() = default;
         /// Constructor to automatically derive the template argument.
-        TreeWidgetBuilder(const ContainerT&)
+        TreeWidgetBuilder(const ElementT&)
         {}
 
         TreeWidgetBuilder(CompareFn compareFn, MakeItemFn makeItemFn, UpdateItemFn updateItemFn = NoUpdate) :
             compareFn(compareFn), makeItemFn(makeItemFn), updateItemFn(updateItemFn)
         {}
-        TreeWidgetBuilder(NameFn nameFn, MakeItemFn makeItemFn, UpdateItemFn updateItemFn = NoUpdate) :
-            compareFn(MakeCompareNameFn(nameFn)), makeItemFn(makeItemFn), updateItemFn(updateItemFn)
+        TreeWidgetBuilder(NameFn nameFn, MakeItemFn makeItemFn, UpdateItemFn updateItemFn = NoUpdate,
+                          int nameColumn = 0) :
+            compareFn(MakeCompareNameFn(nameFn, nameColumn)), makeItemFn(makeItemFn), updateItemFn(updateItemFn)
         {}
 
 
@@ -45,9 +46,9 @@ namespace armarx
         {
             this->compareFn = compareFn;
         }
-        void setNameFn(NameFn nameFn)
+        void setNameFn(NameFn nameFn, int nameColumn = 0)
         {
-            compareFn = MakeCompareNameFn(nameFn);
+            compareFn = MakeCompareNameFn(nameFn, nameColumn);
         }
         void setMakeItemFn(MakeItemFn makeItemFn)
         {
@@ -64,8 +65,31 @@ namespace armarx
         }
 
 
-        template <class ParentT>
-        void updateTree(ParentT* parent, const ContainerT& elements);
+        /**
+         * @brief Update the tree according to the elements iterated over by `iteratorFn`.
+         *
+         * @param parent
+         *  The parent, i.e. the tree widget or a tree widget item.
+         * @param iteratorFn
+         *  Function taking a function `bool fn(const ElementT& element)`
+         *  and calling it for each element in the underlying container, i.e. like:
+         *  void iteratorFn(bool (*elementFn)(const ElementT& element));
+         */
+        template <class ParentT, class IteratorFn>
+        void updateTreeWithIterator(ParentT* parent, IteratorFn&& iteratorFn);
+
+        /**
+         * @brief Update the tree with the iterable container.
+         *
+         * @param parent
+         *  The parent, i.e. the tree widget or a tree widget item.
+         * @param iteratorFn
+         *  Function taking a function `bool fn(const ElementT& element)`
+         *  and calling it for each element in the underlying container, i.e. like:
+         *  void iteratorFn(bool (*elementFn)(const ElementT& element));
+         */
+        template <class ParentT, class ContainerT>
+        void updateTreeWithContainer(ParentT* parent, const ContainerT& elements);
 
 
         /// No update function (default).
@@ -76,7 +100,7 @@ namespace armarx
         }
 
         /// Uses the name for comparison.
-        static CompareFn MakeCompareNameFn(NameFn nameFn);
+        static CompareFn MakeCompareNameFn(NameFn nameFn, int nameColumn);
 
 
     private:
@@ -100,7 +124,7 @@ namespace armarx
     struct MapTreeWidgetBuilder
     {
         using MapT = std::map<KeyT, ValueT>;
-        using Base = TreeWidgetBuilder<MapT>;
+        using Base = TreeWidgetBuilder<typename MapT::value_type>;
         using ElementT = typename Base::ElementT;
 
         using CompareFn = std::function<int(const ElementT& element, QTreeWidgetItem* item)>;
@@ -170,7 +194,7 @@ namespace armarx
         template <class ParentT>
         void updateTree(ParentT* tree, const MapT& elements)
         {
-            builder.updateTree(tree, elements);
+            builder.updateTreeWithContainer(tree, elements);
         }
 
 
@@ -200,7 +224,7 @@ namespace armarx
 
     private:
 
-        TreeWidgetBuilder<MapT> builder;
+        TreeWidgetBuilder<typename MapT::value_type> builder;
 
     };
 
@@ -273,30 +297,52 @@ namespace armarx
     }
 
 
-    template <class ContainerT>
-    auto TreeWidgetBuilder<ContainerT>::MakeCompareNameFn(NameFn nameFn) -> CompareFn
+    template <class ElementT>
+    auto TreeWidgetBuilder<ElementT>::MakeCompareNameFn(NameFn nameFn, int nameColumn) -> CompareFn
     {
-        return [nameFn](const ElementT & element, QTreeWidgetItem * item)
+        return [nameFn, nameColumn](const ElementT & element, QTreeWidgetItem * item)
         {
-            return detail::compare(nameFn(element), item->text(0).toStdString());
+            return detail::compare(nameFn(element), item->text(nameColumn).toStdString());
         };
     }
 
 
-    template <class ContainerT>
-    template <typename ParentT>
-    void TreeWidgetBuilder<ContainerT>::updateTree(ParentT* parent, const ContainerT& elements)
+    template <class ElementT>
+    template <class ParentT, class ContainerT>
+    void TreeWidgetBuilder<ElementT>::updateTreeWithContainer(ParentT* parent, const ContainerT& elements)
+    {
+        this->updateTreeWithIterator(parent, [&elements](auto&& elementFn)
+        {
+            for (const auto& element : elements)
+            {
+                if (not elementFn(element))
+                {
+                    break;
+                }
+            }
+        });
+    }
+
+
+    template <class ElementT>
+    template <class ParentT, class IteratorFn>
+    void TreeWidgetBuilder<ElementT>::updateTreeWithIterator(ParentT* parent, IteratorFn&& iteratorFn)
     {
         using api = detail::ParentAPI<ParentT>;
 
+        ARMARX_CHECK_NOT_NULL(makeItemFn) << "makeItemFn must be set";
+        ARMARX_CHECK_NOT_NULL(updateItemFn) << "updateItemFn must be set";
+        ARMARX_CHECK_NOT_NULL(compareFn) << "compareFn must be set";
+
         int currentIndex = 0;
-        for (const auto& element : elements)
+        iteratorFn([this, &parent, &currentIndex](const auto & element)
         {
             bool inserted = false;
             QTreeWidgetItem* item = nullptr;
             if (currentIndex >= api::getItemCount(parent))
             {
                 // Add elements to the end of the list.
+                ARMARX_CHECK_NOT_NULL(makeItemFn);
                 item = makeItemFn(element);
                 api::insertItem(parent, api::getItemCount(parent), item);
                 ++currentIndex;
@@ -331,11 +377,8 @@ namespace armarx
             {
                 item->setExpanded(true);
             }
-            if (!cont)
-            {
-                break;
-            }
-        }
+            return cont;
+        });
         // Remove superfluous items. (currentIndex must point behind the last item)
         while (api::getItemCount(parent) > currentIndex)
         {
diff --git a/source/RobotAPI/libraries/armem_gui/instance/MemoryIDTreeWidgetItem.cpp b/source/RobotAPI/libraries/armem_gui/instance/MemoryIDTreeWidgetItem.cpp
index b8f568c79..ebb4473bb 100644
--- a/source/RobotAPI/libraries/armem_gui/instance/MemoryIDTreeWidgetItem.cpp
+++ b/source/RobotAPI/libraries/armem_gui/instance/MemoryIDTreeWidgetItem.cpp
@@ -33,7 +33,11 @@ namespace armarx::armem::gui::instance
         // Timestamp in human-readable format
         if (id.hasTimestamp())
         {
-            child(4)->setText(valueColumn, QString::fromStdString(toDateTimeMilliSeconds(id.timestamp)));
+            static const char* mu = "\u03BC";
+            std::stringstream ss;
+            ss << toDateTimeMilliSeconds(id.timestamp)
+               << " (" << id.timestamp.toMicroSeconds() << " " << mu << "s)";
+            child(4)->setText(valueColumn, QString::fromStdString(ss.str()));
         }
     }
 
diff --git a/source/RobotAPI/libraries/armem_gui/instance/tree_builders/DataTreeBuilder.cpp b/source/RobotAPI/libraries/armem_gui/instance/tree_builders/DataTreeBuilder.cpp
index a2811c0bc..6793fd795 100644
--- a/source/RobotAPI/libraries/armem_gui/instance/tree_builders/DataTreeBuilder.cpp
+++ b/source/RobotAPI/libraries/armem_gui/instance/tree_builders/DataTreeBuilder.cpp
@@ -22,7 +22,7 @@ namespace armarx::armem::gui::instance
             return true;
         });
 
-        builder.updateTree(parent, data->getAllKeys());
+        builder.updateTreeWithContainer(parent, data->getAllKeys());
     }
 
     void DataTreeBuilder::updateTree(QTreeWidgetItem* parent, const aron::datanavigator::ListNavigatorPtr& data)
@@ -36,7 +36,7 @@ namespace armarx::armem::gui::instance
             return true;
         });
 
-        builder.updateTree(parent, getIndex(children.size()));
+        builder.updateTreeWithContainer(parent, getIndex(children.size()));
     }
 
 
diff --git a/source/RobotAPI/libraries/armem_gui/instance/tree_builders/DataTreeBuilderBase.h b/source/RobotAPI/libraries/armem_gui/instance/tree_builders/DataTreeBuilderBase.h
index b26a422ea..b225a093d 100644
--- a/source/RobotAPI/libraries/armem_gui/instance/tree_builders/DataTreeBuilderBase.h
+++ b/source/RobotAPI/libraries/armem_gui/instance/tree_builders/DataTreeBuilderBase.h
@@ -28,8 +28,8 @@ namespace armarx::armem::gui::instance
 
     protected:
 
-        using DictBuilder = armarx::TreeWidgetBuilder<std::vector<std::string>>;
-        using ListBuilder = armarx::TreeWidgetBuilder<std::vector<size_t>>;
+        using DictBuilder = armarx::TreeWidgetBuilder<std::string>;
+        using ListBuilder = armarx::TreeWidgetBuilder<size_t>;
 
         DictBuilder getDictBuilder() const;
         ListBuilder getListBuilder() const;
diff --git a/source/RobotAPI/libraries/armem_gui/instance/tree_builders/TypedDataTreeBuilder.cpp b/source/RobotAPI/libraries/armem_gui/instance/tree_builders/TypedDataTreeBuilder.cpp
index 56d204abf..4673f22f6 100644
--- a/source/RobotAPI/libraries/armem_gui/instance/tree_builders/TypedDataTreeBuilder.cpp
+++ b/source/RobotAPI/libraries/armem_gui/instance/tree_builders/TypedDataTreeBuilder.cpp
@@ -40,7 +40,7 @@ namespace armarx::armem::gui::instance
                 return true;
             });
 
-            builder.updateTree(parent, data.getAllKeys());
+            builder.updateTreeWithContainer(parent, data.getAllKeys());
         }
     }
 
@@ -76,7 +76,7 @@ namespace armarx::armem::gui::instance
             return true;
         });
 
-        builder.updateTree(parent, type.getAllKeys());
+        builder.updateTreeWithContainer(parent, type.getAllKeys());
     }
 
 
@@ -100,7 +100,7 @@ namespace armarx::armem::gui::instance
                 return true;
             });
 
-            builder.updateTree(parent, getIndex(children.size()));
+            builder.updateTreeWithContainer(parent, getIndex(children.size()));
         }
     }
 
@@ -126,7 +126,7 @@ namespace armarx::armem::gui::instance
             return true;
         });
 
-        builder.updateTree(parent, getIndex(data.childrenSize()));
+        builder.updateTreeWithContainer(parent, getIndex(data.childrenSize()));
     }
 
 
@@ -150,7 +150,7 @@ namespace armarx::armem::gui::instance
             return true;
         });
 
-        builder.updateTree(parent, getIndex(type.getAcceptedTypes().size()));
+        builder.updateTreeWithContainer(parent, getIndex(type.getAcceptedTypes().size()));
     }
 
 
diff --git a/source/RobotAPI/libraries/armem_gui/memory/TreeWidget.cpp b/source/RobotAPI/libraries/armem_gui/memory/TreeWidget.cpp
index 06ea1c2e0..5c6f363cf 100644
--- a/source/RobotAPI/libraries/armem_gui/memory/TreeWidget.cpp
+++ b/source/RobotAPI/libraries/armem_gui/memory/TreeWidget.cpp
@@ -66,13 +66,18 @@ namespace armarx::armem::gui::memory
             return true;
         });
 
+        auto nameFn = [](const auto & element)
+        {
+            return element.name();
+        };
 
         workingmemoryCoreSegmentBuilder.setExpand(true);
-        workingmemoryCoreSegmentBuilder.setMakeItemFn([this](const std::string & name, const wm::CoreSegment & coreSeg)
+        workingmemoryCoreSegmentBuilder.setNameFn(nameFn, int(Columns::KEY));
+        workingmemoryCoreSegmentBuilder.setMakeItemFn([this](const wm::CoreSegment & coreSeg)
         {
-            return makeItem(name, coreSeg);
+            return makeItem(coreSeg.name(), coreSeg);
         });
-        workingmemoryCoreSegmentBuilder.setUpdateItemFn([this](const std::string&, const wm::CoreSegment & coreSeg, QTreeWidgetItem * coreSegItem)
+        workingmemoryCoreSegmentBuilder.setUpdateItemFn([this](const wm::CoreSegment & coreSeg, QTreeWidgetItem * coreSegItem)
         {
             updateContainerItem(coreSeg, coreSegItem);
             updateChildren(coreSeg, coreSegItem);
@@ -80,11 +85,12 @@ namespace armarx::armem::gui::memory
         });
 
         workingmemoryProvSegmentBuilder.setExpand(true);
-        workingmemoryProvSegmentBuilder.setMakeItemFn([this](const std::string & name, const wm::ProviderSegment & provSeg)
+        workingmemoryProvSegmentBuilder.setNameFn(nameFn, int(Columns::KEY));
+        workingmemoryProvSegmentBuilder.setMakeItemFn([this](const wm::ProviderSegment & provSeg)
         {
-            return makeItem(name, provSeg);
+            return makeItem(provSeg.name(), provSeg);
         });
-        workingmemoryProvSegmentBuilder.setUpdateItemFn([this](const std::string&, const wm::ProviderSegment & provSeg, QTreeWidgetItem * provSegItem)
+        workingmemoryProvSegmentBuilder.setUpdateItemFn([this](const wm::ProviderSegment & provSeg, QTreeWidgetItem * provSegItem)
         {
             updateContainerItem(provSeg, provSegItem);
             updateChildren(provSeg, provSegItem);
@@ -92,30 +98,30 @@ namespace armarx::armem::gui::memory
         });
 
         // entityBuilder.setExpand(true);
-        workingmemoryEntityBuilder.setMakeItemFn([this](const std::string & name, const wm::Entity & entity)
+        workingmemoryEntityBuilder.setNameFn(nameFn, int(Columns::KEY));
+        workingmemoryEntityBuilder.setMakeItemFn([this](const wm::Entity & entity)
         {
-            return makeItem(name, entity);
+            return makeItem(entity.name(), entity);
         });
-        workingmemoryEntityBuilder.setUpdateItemFn([this](const std::string&, const wm::Entity & entity, QTreeWidgetItem *  entityItem)
+        workingmemoryEntityBuilder.setUpdateItemFn([this](const wm::Entity & entity, QTreeWidgetItem *  entityItem)
         {
             updateContainerItem(entity, entityItem);
             updateChildren(entity, entityItem);
             return true;
         });
 
-        workingmemorySnapshotBuilder.setMakeItemFn([this](const armem::Time & time, const wm::EntitySnapshot & snapshot)
+        workingmemorySnapshotBuilder.setMakeItemFn([this](const wm::EntitySnapshot & snapshot)
         {
-            QTreeWidgetItem* item = makeItem(toDateTimeMilliSeconds(time), snapshot);
+            QTreeWidgetItem* item = makeItem(toDateTimeMilliSeconds(snapshot.time()), snapshot);
             item->setData(int(Columns::KEY), Qt::ItemDataRole::UserRole, QVariant(static_cast<qlonglong>(snapshot.time().toMicroSeconds())));
             return item;
         });
-        workingmemorySnapshotBuilder.setCompareFn([](const auto & pair, QTreeWidgetItem * item)
+        workingmemorySnapshotBuilder.setCompareFn([](const wm::EntitySnapshot & snapshot, QTreeWidgetItem * item)
         {
-            const wm::EntitySnapshot& snapshot = pair.second;
             return armarx::detail::compare(static_cast<qlonglong>(snapshot.time().toMicroSeconds()),
                                            item->data(int(Columns::KEY), Qt::ItemDataRole::UserRole).toLongLong());
         });
-        workingmemorySnapshotBuilder.setUpdateItemFn([this](const armem::Time&, const wm::EntitySnapshot & snapshot, QTreeWidgetItem * snapshotItem)
+        workingmemorySnapshotBuilder.setUpdateItemFn([this](const wm::EntitySnapshot & snapshot, QTreeWidgetItem * snapshotItem)
         {
             updateContainerItem(snapshot, snapshotItem);
             updateChildren(snapshot, snapshotItem);
@@ -213,6 +219,16 @@ namespace armarx::armem::gui::memory
         emit selectedItemChanged(*_selectedID);
     }
 
+    template <class ContainerT>
+    static
+    auto makeIteratorFn(const ContainerT& container)
+    {
+        return [&container](auto&& elementFn)
+        {
+            container.forEachChild(elementFn);
+        };
+    }
+
 
     void TreeWidget::updateChildren(const armem::wm::Memory& memory, QTreeWidget* tree)
     {
@@ -227,27 +243,27 @@ namespace armarx::armem::gui::memory
 
     void TreeWidget::updateChildren(const armem::wm::Memory& memory, QTreeWidgetItem* memoryItem)
     {
-        workingmemoryCoreSegmentBuilder.updateTree(memoryItem, memory.coreSegments());
+        workingmemoryCoreSegmentBuilder.updateTreeWithIterator(memoryItem, makeIteratorFn(memory));
     }
 
     void TreeWidget::updateChildren(const armem::wm::CoreSegment& coreSeg, QTreeWidgetItem* coreSegItem)
     {
-        workingmemoryProvSegmentBuilder.updateTree(coreSegItem, coreSeg.providerSegments());
+        workingmemoryProvSegmentBuilder.updateTreeWithIterator(coreSegItem, makeIteratorFn(coreSeg));
     }
 
     void TreeWidget::updateChildren(const armem::wm::ProviderSegment& provSeg, QTreeWidgetItem* provSegItem)
     {
-        workingmemoryEntityBuilder.updateTree(provSegItem, provSeg.entities());
+        workingmemoryEntityBuilder.updateTreeWithIterator(provSegItem, makeIteratorFn(provSeg));
     }
 
     void TreeWidget::updateChildren(const armem::wm::Entity& entity, QTreeWidgetItem* entityItem)
     {
-        workingmemorySnapshotBuilder.updateTree(entityItem, entity.history());
+        workingmemorySnapshotBuilder.updateTreeWithIterator(entityItem, makeIteratorFn(entity));
     }
 
     void TreeWidget::updateChildren(const armem::wm::EntitySnapshot& snapshot, QTreeWidgetItem* snapshotItem)
     {
-        workingmemoryInstanceBuilder.updateTree(snapshotItem, snapshot.instances());
+        workingmemoryInstanceBuilder.updateTreeWithIterator(snapshotItem, makeIteratorFn(snapshot));
     }
 
     void TreeWidget::updateChildren(const armem::wm::EntityInstance& data, QTreeWidgetItem* dataItem)
diff --git a/source/RobotAPI/libraries/armem_gui/memory/TreeWidget.h b/source/RobotAPI/libraries/armem_gui/memory/TreeWidget.h
index ddac9237c..0dda62162 100644
--- a/source/RobotAPI/libraries/armem_gui/memory/TreeWidget.h
+++ b/source/RobotAPI/libraries/armem_gui/memory/TreeWidget.h
@@ -78,13 +78,12 @@ namespace armarx::armem::gui::memory
 
     private:
 
-
         MapTreeWidgetBuilder<std::string, const wm::Memory*> workingmemoryBuilder;
-        MapTreeWidgetBuilder<std::string, wm::CoreSegment> workingmemoryCoreSegmentBuilder;
-        MapTreeWidgetBuilder<std::string, wm::ProviderSegment> workingmemoryProvSegmentBuilder;
-        MapTreeWidgetBuilder<std::string, wm::Entity> workingmemoryEntityBuilder;
-        MapTreeWidgetBuilder<armem::Time, wm::EntitySnapshot> workingmemorySnapshotBuilder;
-        TreeWidgetBuilder<std::vector<wm::EntityInstance>> workingmemoryInstanceBuilder;
+        TreeWidgetBuilder<wm::CoreSegment> workingmemoryCoreSegmentBuilder;
+        TreeWidgetBuilder<wm::ProviderSegment> workingmemoryProvSegmentBuilder;
+        TreeWidgetBuilder<wm::Entity> workingmemoryEntityBuilder;
+        TreeWidgetBuilder<wm::EntitySnapshot> workingmemorySnapshotBuilder;
+        TreeWidgetBuilder<wm::EntityInstance> workingmemoryInstanceBuilder;
 
         std::optional<MemoryID> _selectedID;
         /// While this is false, do not handle selection updates.
-- 
GitLab