From b6b701a2ae68a4e9c388966180f5807c296d52b2 Mon Sep 17 00:00:00 2001
From: Gennady Pospelov <g.pospelov@fz-juelich.de>
Date: Tue, 31 Jan 2017 16:50:59 +0100
Subject: [PATCH] GroupProperty cleanup and methods renaming.

---
 GUI/coregui/Models/GroupItem.cpp              | 14 +++--
 GUI/coregui/Models/GroupItem.h                |  8 ++-
 GUI/coregui/Models/GroupProperty.cpp          | 55 +++++++++----------
 GUI/coregui/Models/GroupProperty.h            | 17 +++---
 GUI/coregui/Models/ModelPath.cpp              |  6 +-
 GUI/coregui/Models/ParameterTreeBuilder.cpp   |  2 +-
 GUI/coregui/Models/SessionItem.cpp            |  8 +--
 GUI/coregui/Models/SessionXML.cpp             |  2 +-
 .../Views/SampleDesigner/ParticleView.cpp     |  4 +-
 Tests/UnitTests/GUI/TestGroupProperty.h       | 14 ++++-
 10 files changed, 71 insertions(+), 59 deletions(-)

diff --git a/GUI/coregui/Models/GroupItem.cpp b/GUI/coregui/Models/GroupItem.cpp
index dfe5e588ab7..bd5a66db178 100644
--- a/GUI/coregui/Models/GroupItem.cpp
+++ b/GUI/coregui/Models/GroupItem.cpp
@@ -34,20 +34,26 @@ void GroupItem::setGroup(GroupProperty_t group)
     setValue(QVariant::fromValue(group));
 }
 
-GroupProperty_t GroupItem::group() const
+GroupProperty_t GroupItem::groupProperty() const
 {
     return value().value<GroupProperty_t>();
 }
 
-SessionItem* GroupItem::currentItem()
+SessionItem* GroupItem::currentItem() const
 {
-    return value().isValid() ? group()->getCurrentItem() : nullptr;
+    return value().isValid() ? groupProperty()->currentItem() : nullptr;
+}
+
+QString GroupItem::currentType() const
+{
+    return groupProperty()->currentType();
 }
 
 SessionItem* GroupItem::setCurrentType(const QString& modelType)
 {
-    GroupProperty_t group_property = group();
+    GroupProperty_t group_property = groupProperty();
     group_property->setCurrentType(modelType);
+
     return currentItem();
 }
 
diff --git a/GUI/coregui/Models/GroupItem.h b/GUI/coregui/Models/GroupItem.h
index ccda647e3f4..4aec85c684c 100644
--- a/GUI/coregui/Models/GroupItem.h
+++ b/GUI/coregui/Models/GroupItem.h
@@ -26,9 +26,11 @@ public:
     static const QString T_ITEMS;
     GroupItem();
 
-    void setGroup(GroupProperty_t group);
-    GroupProperty_t group() const;
-    SessionItem* currentItem();
+    void setGroup(GroupProperty_t groupProperty);
+    GroupProperty_t groupProperty() const;
+    SessionItem* currentItem() const;
+
+    QString currentType() const;
 
     SessionItem* setCurrentType(const QString& modelType);
 
diff --git a/GUI/coregui/Models/GroupProperty.cpp b/GUI/coregui/Models/GroupProperty.cpp
index 093b3aadce5..5fdd0ec33a1 100644
--- a/GUI/coregui/Models/GroupProperty.cpp
+++ b/GUI/coregui/Models/GroupProperty.cpp
@@ -18,53 +18,46 @@
 #include "ItemFactory.h"
 
 GroupProperty::GroupProperty(QString group_name)
-    : m_group_name(std::move(group_name))
-    , m_groupItem(0)
+    : m_group_name(std::move(group_name)), m_groupItem(0)
 {
 }
 
-SessionItem *GroupProperty::getCurrentItem()
+SessionItem* GroupProperty::currentItem()
 {
-    if(m_groupItem) return m_groupItem->getChildByName(this->getCurrentType());
-    return nullptr;
+    return m_groupItem ? m_groupItem->getChildByName(this->currentType()) : nullptr;
 }
 
-void GroupProperty::setGroupItem(SessionItem *groupItem)
+void GroupProperty::setGroupItem(SessionItem* groupItem)
 {
     Q_ASSERT(groupItem);
     m_groupItem = groupItem;
-    SessionItem *item = createCorrespondingItem();
+    SessionItem* item = createCorrespondingItem();
     m_groupItem->insertItem(-1, item);
 }
 
-SessionItem *GroupProperty::createCorrespondingItem()
-{
-    SessionItem *result = ItemFactory::createItem(getCurrentType());
-    return result;
-}
-
-QString GroupProperty::getCurrentType() const
+QString GroupProperty::currentType() const
 {
     return m_current_type;
 }
 
-void GroupProperty::setCurrentType(const QString &type)
+void GroupProperty::setCurrentType(const QString& type)
 {
-    if(type == getCurrentType()) return;
+    if (type == currentType())
+        return;
 
-    SessionItem *prevItem = getCurrentItem();
+    SessionItem* prevItem = currentItem();
     m_current_type = type;
 
-    if(m_groupItem) {
+    if (m_groupItem) {
         if (auto item = m_groupItem->getChildByName(m_current_type)) {
             item->setVisible(true);
             item->setEnabled(true);
         } else {
-            SessionItem *new_item = createCorrespondingItem();
+            SessionItem* new_item = createCorrespondingItem();
             m_groupItem->insertItem(-1, new_item);
         }
 
-        if(prevItem) {
+        if (prevItem) {
             prevItem->setVisible(false);
             prevItem->setEnabled(false);
         }
@@ -81,18 +74,18 @@ QString GroupProperty::getCurrentLabel() const
 QStringList GroupProperty::getTypes() const
 {
     QStringList result;
-    for (const auto& key_value_pair : m_type_label_map) {
+    for (const auto& key_value_pair : m_type_label_map)
         result << key_value_pair.first;
-    }
+
     return result;
 }
 
 QStringList GroupProperty::getLabels() const
 {
     QStringList result;
-    for (const auto& key_value_pair : m_type_label_map) {
+    for (const auto& key_value_pair : m_type_label_map)
         result << key_value_pair.second;
-    }
+
     return result;
 }
 
@@ -101,13 +94,12 @@ int GroupProperty::index() const
     return toIndex(m_current_type);
 }
 
-int GroupProperty::toIndex(const QString &type) const
+int GroupProperty::toIndex(const QString& type) const
 {
     QStringList name_list = getTypes();
     for (int i = 0; i < name_list.size(); ++i) {
-        if (type == name_list[i]) {
+        if (type == name_list[i])
             return i;
-        }
     }
     return -1;
 }
@@ -115,9 +107,9 @@ int GroupProperty::toIndex(const QString &type) const
 QString GroupProperty::toString(int index) const
 {
     QStringList name_list = getTypes();
-    if (index<0 || index>=name_list.size()) {
+    if (index < 0 || index >= name_list.size())
         return QString();
-    }
+
     return name_list[index];
 }
 
@@ -126,3 +118,8 @@ void GroupProperty::setGroupMap(std::map<QString, QString> group_map)
     m_type_label_map = std::move(group_map);
     setCurrentType(m_type_label_map.begin()->first);
 }
+
+SessionItem* GroupProperty::createCorrespondingItem()
+{
+    return ItemFactory::createItem(currentType());
+}
diff --git a/GUI/coregui/Models/GroupProperty.h b/GUI/coregui/Models/GroupProperty.h
index e6afc6302ce..7717a833b33 100644
--- a/GUI/coregui/Models/GroupProperty.h
+++ b/GUI/coregui/Models/GroupProperty.h
@@ -29,14 +29,12 @@ class SessionItem;
 class BA_CORE_API_ GroupProperty
 {
 public:
-    void setGroupItem(SessionItem *groupItem);
+    void setGroupItem(SessionItem* groupItem);
 
-    SessionItem *getCurrentItem();
+    SessionItem* currentItem();
 
-    SessionItem *createCorrespondingItem();
-
-    QString getCurrentType() const;
-    void setCurrentType(const QString &type);
+    QString currentType() const;
+    void setCurrentType(const QString& type);
 
     QString getCurrentLabel() const;
 
@@ -44,7 +42,7 @@ public:
     QStringList getLabels() const;
 
     int index() const;
-    int toIndex(const QString &type) const;
+    int toIndex(const QString& type) const;
     QString toString(int index) const;
 
     friend class GroupPropertyRegistry;
@@ -52,11 +50,12 @@ public:
 private:
     GroupProperty(QString group_name);
     void setGroupMap(std::map<QString, QString> group_map);
+    SessionItem* createCorrespondingItem();
 
     QString m_group_name;
-    SessionItem *m_groupItem;
+    SessionItem* m_groupItem;
     QString m_current_type;
-    std::map<QString, QString > m_type_label_map;
+    std::map<QString, QString> m_type_label_map;
 };
 
 typedef QSharedPointer<GroupProperty> GroupProperty_t;
diff --git a/GUI/coregui/Models/ModelPath.cpp b/GUI/coregui/Models/ModelPath.cpp
index 83367d020d1..e2168400d6a 100644
--- a/GUI/coregui/Models/ModelPath.cpp
+++ b/GUI/coregui/Models/ModelPath.cpp
@@ -40,7 +40,7 @@ QStringList ModelPath::getParameterTreeList(const SessionItem *item, QString pre
                 if(p_child->isVisible()) {
                     if (p_child->modelType() ==  Constants::GroupItemType) {
                         if (const GroupItem *groupItem = dynamic_cast<const GroupItem*>(p_child)) {
-                            if (const SessionItem *subItem = groupItem->group()->getCurrentItem()) {
+                            if (const SessionItem *subItem = groupItem->currentItem()) {
                                 QString child_prefix = prefix + subItem->itemName() + QString("/");
                                 result << getParameterTreeList(subItem, child_prefix);
                             }
@@ -215,8 +215,8 @@ SessionItem* ModelPath::findChild(const SessionItem *item, const QString& first_
         auto groupItems = item->getChildrenOfType(Constants::GroupItemType);
         for (SessionItem* groupItem : groupItems) {
             if (GroupItem *gItem = dynamic_cast<GroupItem*>(groupItem)) {
-                if (gItem->group()->getCurrentType() == first_field) {
-                    p_child = gItem->group()->getCurrentItem();
+                if (gItem->currentType() == first_field) {
+                    p_child = gItem->currentItem();
                     break;
                 }
             }
diff --git a/GUI/coregui/Models/ParameterTreeBuilder.cpp b/GUI/coregui/Models/ParameterTreeBuilder.cpp
index d58f54e9382..ffa2a3760a5 100644
--- a/GUI/coregui/Models/ParameterTreeBuilder.cpp
+++ b/GUI/coregui/Models/ParameterTreeBuilder.cpp
@@ -138,7 +138,7 @@ void ParameterTreeBuilder::handleItem(SessionItem *tree, SessionItem *source)
 
             } else if (child->modelType() == Constants::GroupItemType) {
                 SessionItem *currentItem
-                    = dynamic_cast<GroupItem *>(child)->group()->getCurrentItem();
+                    = dynamic_cast<GroupItem *>(child)->currentItem();
                 if (currentItem && currentItem->rowCount() > 0) {
                     SessionItem *branch = tree->model()->insertNewItem(
                         Constants::ParameterLabelType, tree->index());
diff --git a/GUI/coregui/Models/SessionItem.cpp b/GUI/coregui/Models/SessionItem.cpp
index ccdeb26a71b..e05ef6a6255 100644
--- a/GUI/coregui/Models/SessionItem.cpp
+++ b/GUI/coregui/Models/SessionItem.cpp
@@ -483,13 +483,13 @@ SessionItem *SessionItem::addGroupProperty(const QString &groupName, const QStri
 SessionItem *SessionItem::getGroupItem(const QString &name, const QString &type) const
 {
     if (GroupItem *item = dynamic_cast<GroupItem *>(getItem(name))) {
-        GroupProperty_t group_property = item->group();
+        GroupProperty_t group_property = item->groupProperty();
         if (type.isEmpty()) {
-            return group_property->getCurrentItem();
+            return group_property->currentItem();
         }
-        QString backup = group_property->getCurrentType();
+        QString backup = group_property->currentType();
         group_property->setCurrentType(type);
-        SessionItem *value = group_property->getCurrentItem();
+        SessionItem *value = group_property->currentItem();
         group_property->setCurrentType(backup);
         return value;
     }
diff --git a/GUI/coregui/Models/SessionXML.cpp b/GUI/coregui/Models/SessionXML.cpp
index 30584364fb7..bd8fcc2ce06 100644
--- a/GUI/coregui/Models/SessionXML.cpp
+++ b/GUI/coregui/Models/SessionXML.cpp
@@ -121,7 +121,7 @@ void SessionWriter::writeVariant(QXmlStreamWriter *writer, QVariant variant, int
         }
 
         else if (type_name == Constants::GroupPropertyType) {
-            QString ff_name = variant.value<GroupProperty_t>()->getCurrentType();
+            QString ff_name = variant.value<GroupProperty_t>()->currentType();
             writer->writeAttribute(SessionXML::ParameterValueAttribute, ff_name);
         }
 
diff --git a/GUI/coregui/Views/SampleDesigner/ParticleView.cpp b/GUI/coregui/Views/SampleDesigner/ParticleView.cpp
index 2e08f39738c..4c91ed2ab63 100644
--- a/GUI/coregui/Views/SampleDesigner/ParticleView.cpp
+++ b/GUI/coregui/Views/SampleDesigner/ParticleView.cpp
@@ -101,8 +101,8 @@ void ParticleView::updatePixmap()
     GroupProperty_t group_property
         = dynamic_cast<GroupItem *>(
               getItem()->getItem(ParticleItem::P_FORM_FACTOR))
-              ->group();
-    QString current_ff_type = group_property->getCurrentType();
+              ->groupProperty();
+    QString current_ff_type = group_property->currentType();
     QString filename
         = QString(":/widgetbox/images/ff_%1_32.png").arg(current_ff_type);
     m_pixmap = QPixmap(filename);
diff --git a/Tests/UnitTests/GUI/TestGroupProperty.h b/Tests/UnitTests/GUI/TestGroupProperty.h
index 6560bb2edb7..a3009cae062 100644
--- a/Tests/UnitTests/GUI/TestGroupProperty.h
+++ b/Tests/UnitTests/GUI/TestGroupProperty.h
@@ -17,17 +17,19 @@ inline void TestGroupProperty::test_CreateGroup()
     GroupProperty_t property = GroupPropertyRegistry::createGroupProperty(
         "MyGroupProperty", Constants::DistributionGroup);
 
-    QCOMPARE(property->getCurrentType(), Constants::DistributionCosineType);
+    QCOMPARE(property->currentType(), Constants::DistributionCosineType);
 
     GroupItem groupItem;
     QCOMPARE(groupItem.childItems().size(), 0);
     QVERIFY(groupItem.currentItem() == nullptr);
 
-    // setting group property
+    // setting group property and checking currentItem
     groupItem.setGroup(property);
     QCOMPARE(groupItem.childItems().size(), 1);
     QCOMPARE(groupItem.childItems()[0], groupItem.currentItem());
-    QCOMPARE(groupItem.currentItem()->modelType(), Constants::DistributionCosineType);
+    SessionItem *cosineItem = groupItem.currentItem();
+    QCOMPARE(cosineItem->modelType(), Constants::DistributionCosineType);
+    QCOMPARE(property->currentItem(), cosineItem);
 
     // setting group property twice
     QVERIFY_THROW(groupItem.setGroup(property), GUIHelpers::Error);
@@ -38,6 +40,12 @@ inline void TestGroupProperty::test_CreateGroup()
     QCOMPARE(newItem->modelType(), Constants::DistributionNoneType);
     QCOMPARE(groupItem.childItems().size(), 2);
 
+    // returning back to previous item
+    QCOMPARE(groupItem.setCurrentType(Constants::DistributionCosineType), cosineItem);
+    QCOMPARE(groupItem.currentItem(), cosineItem);
+    QCOMPARE(groupItem.childItems().size(), 2);
+
+
 }
 
 //GroupProperty_t group_property = item->group();
-- 
GitLab