From a8150ed1e9bfb5801f13e24ca303148d9863a66e Mon Sep 17 00:00:00 2001
From: Rainer Kartmann <rainer.kartmann@kit.edu>
Date: Thu, 26 Aug 2021 18:34:12 +0200
Subject: [PATCH] Improve handling of ARON optionals

---
 .../armem_gui/instance/InstanceView.cpp       | 47 ++++++-----
 .../armem_gui/instance/InstanceView.h         |  2 +-
 .../tree_builders/DataTreeBuilder.cpp         | 26 ++++---
 .../tree_builders/TypedDataTreeBuilder.cpp    | 77 +++++++++++--------
 .../tree_builders/TypedDataTreeBuilder.h      |  2 +-
 5 files changed, 88 insertions(+), 66 deletions(-)

diff --git a/source/RobotAPI/libraries/armem_gui/instance/InstanceView.cpp b/source/RobotAPI/libraries/armem_gui/instance/InstanceView.cpp
index 0cbc6c164..8fda3cc4b 100644
--- a/source/RobotAPI/libraries/armem_gui/instance/InstanceView.cpp
+++ b/source/RobotAPI/libraries/armem_gui/instance/InstanceView.cpp
@@ -225,11 +225,18 @@ namespace armarx::armem::gui::instance
     }
 
 
-    aron::Path InstanceView::getElementPath(const QTreeWidgetItem* item) const
+    std::optional<aron::Path> InstanceView::getElementPath(const QTreeWidgetItem* item) const
     {
         QStringList qpath = item->data(int(Columns::KEY), Qt::UserRole).toStringList();
-        aron::Path path = deserializePath(qpath);
-        return path;
+        if (qpath.empty())
+        {
+            return std::nullopt;
+        }
+        else
+        {
+            aron::Path path = deserializePath(qpath);
+            return path;
+        }
     }
 
 
@@ -261,14 +268,15 @@ namespace armarx::armem::gui::instance
         {
             case aron::type::Descriptor::eIVTCByteImage:
             {
-                const aron::Path path = getElementPath(item);
-
-                QAction* viewAction = new QAction("Show image");
-                menu.addAction(viewAction);
-                connect(viewAction, &QAction::triggered, [this, path]()
+                if (const std::optional<aron::Path> path = getElementPath(item))
                 {
-                    this->showImageView(path);
-                });
+                    QAction* viewAction = new QAction("Show image");
+                    menu.addAction(viewAction);
+                    connect(viewAction, &QAction::triggered, [this, path]()
+                    {
+                        this->showImageView(path.value());
+                    });
+                }
             }
             break;
             default:
@@ -279,17 +287,18 @@ namespace armarx::armem::gui::instance
         const std::string typeName = item->text(int(Columns::TYPE)).toStdString();
         if (typeName == instance::sanitizedMemoryIDTypeName)
         {
-            const aron::Path path = getElementPath(item);
-
-            if (std::optional<MemoryID> id = getElementMemoryID(path))
+            if (const std::optional<aron::Path> path = getElementPath(item))
             {
-                if (QAction* action = makeActionCopyMemoryID(id.value()))
-                {
-                    menu.addAction(action);
-                }
-                if (QAction* action = makeActionResolveMemoryID(id.value()))
+                if (std::optional<MemoryID> id = getElementMemoryID(path.value()))
                 {
-                    menu.addAction(action);
+                    if (QAction* action = makeActionCopyMemoryID(id.value()))
+                    {
+                        menu.addAction(action);
+                    }
+                    if (QAction* action = makeActionResolveMemoryID(id.value()))
+                    {
+                        menu.addAction(action);
+                    }
                 }
             }
         }
diff --git a/source/RobotAPI/libraries/armem_gui/instance/InstanceView.h b/source/RobotAPI/libraries/armem_gui/instance/InstanceView.h
index 78e6ad4ef..76a3988db 100644
--- a/source/RobotAPI/libraries/armem_gui/instance/InstanceView.h
+++ b/source/RobotAPI/libraries/armem_gui/instance/InstanceView.h
@@ -71,7 +71,7 @@ namespace armarx::armem::gui::instance
 
         void showErrorMessage(const std::string& message);
 
-        aron::Path getElementPath(const QTreeWidgetItem* item) const;
+        std::optional<aron::Path> getElementPath(const QTreeWidgetItem* item) const;
         std::optional<MemoryID> getElementMemoryID(const aron::Path& elementPath);
 
         QAction* makeActionResolveMemoryID(const MemoryID& id);
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 6793fd795..bdf6aed5b 100644
--- a/source/RobotAPI/libraries/armem_gui/instance/tree_builders/DataTreeBuilder.cpp
+++ b/source/RobotAPI/libraries/armem_gui/instance/tree_builders/DataTreeBuilder.cpp
@@ -42,21 +42,23 @@ namespace armarx::armem::gui::instance
 
     void DataTreeBuilder::update(QTreeWidgetItem* item, const std::string& key, const aron::datanavigator::NavigatorPtr& data)
     {
-        if (!data)
+        if (data)
         {
-            this->setRowTexts(item, key, "null");
-            return;
+            this->setRowTexts(item, key, *data);
+
+            if (auto cast = aron::datanavigator::DictNavigator::DynamicCast(data))
+            {
+                updateTree(item, cast);
+            }
+            else if (auto cast = aron::datanavigator::ListNavigator::DynamicCast(data))
+            {
+                updateTree(item, cast);
+            }
         }
-
-        this->setRowTexts(item, key, *data);
-
-        if (auto cast = aron::datanavigator::DictNavigator::DynamicCast(data))
-        {
-            updateTree(item, cast);
-        }
-        else if (auto cast = aron::datanavigator::ListNavigator::DynamicCast(data))
+        else
         {
-            updateTree(item, cast);
+            // Optional data?
+            this->setRowTexts(item, key, "(none)");
         }
     }
 
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 c234ada08..bda945607 100644
--- a/source/RobotAPI/libraries/armem_gui/instance/tree_builders/TypedDataTreeBuilder.cpp
+++ b/source/RobotAPI/libraries/armem_gui/instance/tree_builders/TypedDataTreeBuilder.cpp
@@ -36,7 +36,7 @@ namespace armarx::armem::gui::instance
                 auto childData = data.getElement(key);
                 if (childData)
                 {
-                    this->update(item, key, *childType, *childData);
+                    this->update(item, key, *childType, childData.get());
                 }
                 return true;
             });
@@ -70,9 +70,9 @@ namespace armarx::armem::gui::instance
             auto childType = type.getMemberType(key);
             auto childData = data.getElement(key);
 
-            if (childType && childData)
+            if (childType)
             {
-                this->update(item, key, *childType, *childData);
+                this->update(item, key, *childType, childData.get());
             }
             return true;
         });
@@ -93,10 +93,9 @@ namespace armarx::armem::gui::instance
             ListBuilder builder = getListBuilder();
             builder.setUpdateItemFn([this, &children, &childType](size_t key, QTreeWidgetItem * item)
             {
-                auto childData = children.at(key);
-                if (childData)
+                if (auto childData = children.at(key))
                 {
-                    this->update(item, std::to_string(key), *childType, *childData);
+                    this->update(item, std::to_string(key), *childType, childData.get());
                 }
                 return true;
             });
@@ -120,9 +119,9 @@ namespace armarx::armem::gui::instance
             auto childType = i == 0 ? childTypes.first : childTypes.second;
             auto childData = data.getElement(static_cast<unsigned int>(i));
 
-            if (childType && childData)
+            if (childType)
             {
-                this->update(item, std::to_string(i), *childType, *childData);
+                this->update(item, std::to_string(i), *childType, childData.get());
             }
             return true;
         });
@@ -144,9 +143,9 @@ namespace armarx::armem::gui::instance
             auto childType = childTypes.at(i);
             auto childData = data.getElement(static_cast<unsigned int>(i));
 
-            if (childType && childData)
+            if (childType)
             {
-                this->update(item, std::to_string(i), *childType, *childData);
+                this->update(item, std::to_string(i), *childType, childData.get());
             }
             return true;
         });
@@ -159,21 +158,30 @@ namespace armarx::armem::gui::instance
         QTreeWidgetItem* item,
         const std::string& key,
         aron::typenavigator::Navigator& type,
-        aron::datanavigator::Navigator& data)
+        aron::datanavigator::Navigator* data)
     {
         using namespace aron;
 
-        const std::string value = aron::TypedDataDisplayVisitor::getValue(type, data);
-        const std::string typeName = instance::sanitizeTypeName(type.getName());
+        const std::string value = data ? aron::TypedDataDisplayVisitor::getValue(type, *data) : "(none)";
+        std::string typeName = instance::sanitizeTypeName(type.getName());
+        switch (type.getMaybe())
+        {
+            case aron::type::Maybe::eOptional:
+                typeName = "Optional[" + typeName + "]";
+                break;
+            default:
+                break;
+        }
+
         setRowTexts(item, key, value, typeName);
 
-        item->setData(columnKey, Qt::UserRole, instance::serializePath(data.getPath()));
+        item->setData(columnKey, Qt::UserRole, data ? instance::serializePath(data->getPath()) : QStringList());
         item->setData(columnType, Qt::UserRole, static_cast<int>(type.getDescriptor()));
 
         if (typeName == sanitizedMemoryIDTypeName)
         {
             MemoryIDTreeWidgetItem* memoryIDItem = dynamic_cast<MemoryIDTreeWidgetItem*>(item);
-            datanavigator::DictNavigator* dictData = dynamic_cast<datanavigator::DictNavigator*>(&data);
+            datanavigator::DictNavigator* dictData = dynamic_cast<datanavigator::DictNavigator*>(data);
             if (memoryIDItem && dictData)
             {
                 arondto::MemoryID dto;
@@ -186,25 +194,28 @@ namespace armarx::armem::gui::instance
             }
         }
 
-        if (auto t = dynamic_cast<typenavigator::ObjectNavigator*>(&type))
-        {
-            _updateTree<datanavigator::DictNavigator>(item, *t, data);
-        }
-        else if (auto t = dynamic_cast<typenavigator::DictNavigator*>(&type))
-        {
-            _updateTree<datanavigator::DictNavigator>(item, *t, data);
-        }
-        else if (auto t = dynamic_cast<typenavigator::ListNavigator*>(&type))
-        {
-            _updateTree<datanavigator::ListNavigator>(item, *t, data);
-        }
-        else if (auto t = dynamic_cast<typenavigator::PairNavigator*>(&type))
-        {
-            _updateTree<datanavigator::ListNavigator>(item, *t, data);
-        }
-        else if (auto t = dynamic_cast<typenavigator::TupleNavigator*>(&type))
+        if (data)
         {
-            _updateTree<datanavigator::ListNavigator>(item, *t, data);
+            if (auto t = dynamic_cast<typenavigator::ObjectNavigator*>(&type))
+            {
+                _updateTree<datanavigator::DictNavigator>(item, *t, *data);
+            }
+            else if (auto t = dynamic_cast<typenavigator::DictNavigator*>(&type))
+            {
+                _updateTree<datanavigator::DictNavigator>(item, *t, *data);
+            }
+            else if (auto t = dynamic_cast<typenavigator::ListNavigator*>(&type))
+            {
+                _updateTree<datanavigator::ListNavigator>(item, *t, *data);
+            }
+            else if (auto t = dynamic_cast<typenavigator::PairNavigator*>(&type))
+            {
+                _updateTree<datanavigator::ListNavigator>(item, *t, *data);
+            }
+            else if (auto t = dynamic_cast<typenavigator::TupleNavigator*>(&type))
+            {
+                _updateTree<datanavigator::ListNavigator>(item, *t, *data);
+            }
         }
     }
 
diff --git a/source/RobotAPI/libraries/armem_gui/instance/tree_builders/TypedDataTreeBuilder.h b/source/RobotAPI/libraries/armem_gui/instance/tree_builders/TypedDataTreeBuilder.h
index 57580a1f6..0dead6736 100644
--- a/source/RobotAPI/libraries/armem_gui/instance/tree_builders/TypedDataTreeBuilder.h
+++ b/source/RobotAPI/libraries/armem_gui/instance/tree_builders/TypedDataTreeBuilder.h
@@ -52,7 +52,7 @@ namespace armarx::armem::gui::instance
         void update(QTreeWidgetItem* item,
                     const std::string& key,
                     aron::typenavigator::Navigator& type,
-                    aron::datanavigator::Navigator& data);
+                    aron::datanavigator::Navigator* data);
 
         template <class DataT, class TypeT>
         void _updateTree(QTreeWidgetItem* item, TypeT& type, aron::datanavigator::Navigator& data);
-- 
GitLab