From cf67e25241ade17e5a40c4ac9f9c8ee977a525cd Mon Sep 17 00:00:00 2001
From: Gennady Pospelov <g.pospelov@fz-juelich.de>
Date: Fri, 3 Feb 2017 16:51:44 +0100
Subject: [PATCH] ComboProperty's GUICache value is removed.

---
 GUI/coregui/Models/ComboProperty.cpp          |  11 +-
 GUI/coregui/Models/ComboProperty.h            |  18 +--
 .../Models/ParticleDistributionItem.cpp       | 110 +++++++-----------
 GUI/coregui/Models/ParticleDistributionItem.h |   4 +-
 GUI/coregui/Models/TransformFromDomain.cpp    |  16 +--
 Tests/UnitTests/GUI/TestComboProperty.h       |  10 ++
 6 files changed, 68 insertions(+), 101 deletions(-)

diff --git a/GUI/coregui/Models/ComboProperty.cpp b/GUI/coregui/Models/ComboProperty.cpp
index a1180637d10..bf900835ca5 100644
--- a/GUI/coregui/Models/ComboProperty.cpp
+++ b/GUI/coregui/Models/ComboProperty.cpp
@@ -16,18 +16,22 @@
 
 #include "ComboProperty.h"
 #include "GUIHelpers.h"
-
+#include <QDebug>
 
 ComboProperty::ComboProperty(const QStringList &values, const QString &current_value)
     : m_values(values)
     , m_current_value(current_value)
-    , m_cache_contains_GUI_value(true)
 {
+    if(!m_values.contains(m_current_value))
+        throw GUIHelpers::Error("ComboProperty::ComboProperty() -> Error. Attempt to construct "
+                                "property with initial values not from the list.");
 }
 
 void ComboProperty::setValue(const QString &name)
 {
-    Q_ASSERT(m_values.contains(name));
+    if(!m_values.contains(name))
+        throw GUIHelpers::Error("ComboProperty::setValue() -> Error. Combo doesn't contain "
+                                "value " + name);
     m_current_value = name;
 }
 
@@ -76,7 +80,6 @@ bool ComboProperty::operator==(const ComboProperty &other) const {
     if(m_current_value != other.m_current_value) return false;
     if(!GUIHelpers::isTheSame(m_values, other.m_values)) return false;
     if(m_cached_value != other.m_cached_value) return false;
-    if(m_cache_contains_GUI_value != other.m_cache_contains_GUI_value) return false;
     return true;
 }
 
diff --git a/GUI/coregui/Models/ComboProperty.h b/GUI/coregui/Models/ComboProperty.h
index f4597f59803..2f03fbf446c 100644
--- a/GUI/coregui/Models/ComboProperty.h
+++ b/GUI/coregui/Models/ComboProperty.h
@@ -29,8 +29,8 @@ class BA_CORE_API_ ComboProperty
 {
 public:
 
-    ComboProperty(const QStringList &values = QStringList(),
-                  const QString &current_value = QString("Undefined"));
+    ComboProperty(){}
+    ComboProperty(const QStringList &values, const QString &current_value);
     virtual ~ComboProperty() {}
     QString getValue() const;
 
@@ -57,9 +57,6 @@ public:
     QString getCachedValue() const;
     void setCachedValue(const QString &name);
 
-    bool cacheContainsGUIValue() const;
-    void setCacheContainsGUIFlag(bool flag=true);
-
     bool operator==(const ComboProperty &other) const;
     bool operator!=(const ComboProperty &other) const { return !(*this == other); }
     bool operator<(const ComboProperty &other) const;
@@ -69,7 +66,6 @@ private:
     QStringList m_values_tooltips;
     QString m_current_value;
     QString m_cached_value;  // for comboboxes with dynamically generated value lists
-    bool m_cache_contains_GUI_value;
 };
 
 inline QString ComboProperty::getValue() const
@@ -117,16 +113,6 @@ inline QString ComboProperty::getCachedValue() const
     return m_cached_value;
 }
 
-inline bool ComboProperty::cacheContainsGUIValue() const
-{
-    return m_cache_contains_GUI_value;
-}
-
-inline void ComboProperty::setCacheContainsGUIFlag(bool flag)
-{
-    m_cache_contains_GUI_value = flag;
-}
-
 Q_DECLARE_METATYPE(ComboProperty)
 
 
diff --git a/GUI/coregui/Models/ParticleDistributionItem.cpp b/GUI/coregui/Models/ParticleDistributionItem.cpp
index 5578063fa42..875037fa83b 100644
--- a/GUI/coregui/Models/ParticleDistributionItem.cpp
+++ b/GUI/coregui/Models/ParticleDistributionItem.cpp
@@ -19,8 +19,6 @@
 #include "DistributionItems.h"
 #include "Distributions.h"
 #include "GUIHelpers.h"
-#include "ModelPath.h"
-#include "ParameterPool.h"
 #include "ParticleItem.h"
 #include "TransformFromDomain.h"
 #include "TransformToDomain.h"
@@ -40,20 +38,20 @@ ParticleDistributionItem::ParticleDistributionItem()
 
     addGroupProperty(P_DISTRIBUTION, Constants::DistributionGroup);
 
-    registerTag(T_PARTICLES, 0, 1, QStringList() << Constants::ParticleType <<
-                Constants::ParticleCoreShellType << Constants::ParticleCompositionType);
+    registerTag(T_PARTICLES, 0, 1, QStringList() << Constants::ParticleType
+                                                 << Constants::ParticleCoreShellType
+                                                 << Constants::ParticleCompositionType);
     setDefaultTag(T_PARTICLES);
 
     ComboProperty par_prop;
     addProperty(P_DISTRIBUTED_PARAMETER, par_prop.getVariant());
     updateParameterList();
-    mapper()->setOnAnyChildChange(
-        [this] (SessionItem* item) {
-            // prevent infinit loop when item changes its own properties
-            if (item && item->modelType()== Constants::PropertyType && item->parent() == this)
-                return;
-            updateParameterList();
-        } );
+    mapper()->setOnAnyChildChange([this](SessionItem* item) {
+        // prevent infinit loop when item changes its own properties
+        if (item && item->modelType() == Constants::PropertyType && item->parent() == this)
+            return;
+        updateParameterList();
+    });
 }
 
 std::unique_ptr<ParticleDistribution> ParticleDistributionItem::createParticleDistribution() const
@@ -70,17 +68,16 @@ std::unique_ptr<ParticleDistribution> ParticleDistributionItem::createParticleDi
 
     auto P_distribution = TransformToDomain::createDistribution(*distr_item);
 
-    auto prop = getItemValue(ParticleDistributionItem::P_DISTRIBUTED_PARAMETER)
-                    .value<ComboProperty>();
+    auto prop
+        = getItemValue(ParticleDistributionItem::P_DISTRIBUTED_PARAMETER).value<ComboProperty>();
     QString par_name = prop.getValue();
 
-    std::string domain_par = ParameterTreeUtils::parameterNameToDomainName(
-        par_name, getItems(T_PARTICLES).front()).toStdString();
+    std::string domain_par
+        = ParameterTreeUtils::parameterNameToDomainName(par_name, getItems(T_PARTICLES).front())
+              .toStdString();
 
-    int nbr_samples
-        = distr_item->getItemValue(DistributionItem::P_NUMBER_OF_SAMPLES).toInt();
-    double sigma_factor
-        = distr_item->getItemValue(DistributionItem::P_SIGMA_FACTOR).toDouble();
+    int nbr_samples = distr_item->getItemValue(DistributionItem::P_NUMBER_OF_SAMPLES).toInt();
+    double sigma_factor = distr_item->getItemValue(DistributionItem::P_SIGMA_FACTOR).toDouble();
     ParameterDistribution par_distr(domain_par, *P_distribution, nbr_samples, sigma_factor);
     auto result = GUIHelpers::make_unique<ParticleDistribution>(*P_particle, par_distr);
     double abundance = getItemValue(ParticleItem::P_ABUNDANCE).toDouble();
@@ -92,37 +89,26 @@ void ParticleDistributionItem::updateParameterList()
 {
     if (!isTag(P_DISTRIBUTED_PARAMETER))
         return;
-    QVariant par_prop = getItemValue(P_DISTRIBUTED_PARAMETER);
-    auto combo_prop = par_prop.value<ComboProperty>();
-    QString cached_par = domainCacheName();
-
-    if (!cacheContainsGUIValue()) {
-        auto gui_name = translateParameterNameToGUI(domainCacheName());
-        if (!gui_name.isEmpty()) {
-            cached_par = gui_name;
-            combo_prop.setCachedValue(cached_par);
-            combo_prop.setCacheContainsGUIFlag();
-            setDomainCacheName(QString());
-        }
-    }
 
-    QString selected_par = combo_prop.getValue();
+    ComboProperty prop = getItemValue(P_DISTRIBUTED_PARAMETER).value<ComboProperty>();
+    QString currentValue = prop.getValue();
 
     QStringList par_names = QStringList() << NO_SELECTION << childParameterNames();
     par_names.removeAll(ParticleItem::P_ABUNDANCE);
+    ComboProperty newProp = ComboProperty(par_names, NO_SELECTION);
 
-    auto updated_prop = ComboProperty(par_names);
-    updated_prop.setCachedValue(cached_par);
-    updated_prop.setCacheContainsGUIFlag(combo_prop.cacheContainsGUIValue());
-
-    if (updated_prop.getValues().contains(cached_par) ) {
-        updated_prop.setValue(cached_par);
-    } else if (updated_prop.getValues().contains(selected_par)) {
-        updated_prop.setValue(selected_par);
-    } else {
-        updated_prop.setValue(NO_SELECTION);
+    if (!m_domain_cache_name.isEmpty()) {
+        QString guiName = translateParameterNameToGUI(m_domain_cache_name);
+        if (!guiName.isEmpty()) { // might be empty because item was not fully constructed yet
+            currentValue = guiName;
+            m_domain_cache_name.clear();
+        }
     }
-    setItemValue(P_DISTRIBUTED_PARAMETER, updated_prop.getVariant());
+
+    if (newProp.getValues().contains(currentValue))
+        newProp.setValue(currentValue);
+
+    setItemValue(P_DISTRIBUTED_PARAMETER, newProp.getVariant());
 }
 
 void ParticleDistributionItem::setDomainCacheName(const QString& name)
@@ -130,37 +116,27 @@ void ParticleDistributionItem::setDomainCacheName(const QString& name)
     m_domain_cache_name = name;
 }
 
-bool ParticleDistributionItem::cacheContainsGUIValue() const
+QStringList ParticleDistributionItem::childParameterNames() const
 {
-    QVariant par_prop = getItemValue(P_DISTRIBUTED_PARAMETER);
-    auto combo_prop = par_prop.value<ComboProperty>();
-    return combo_prop.cacheContainsGUIValue();
-}
+    if(auto child = childParticle())
+        return ParameterTreeUtils::parameterTreeNames(child);
 
-QString ParticleDistributionItem::domainCacheName() const
-{
-    QVariant par_prop = getItemValue(P_DISTRIBUTED_PARAMETER);
-    auto combo_prop = par_prop.value<ComboProperty>();
-    return combo_prop.getCachedValue();
+    return {};
 }
 
-QStringList ParticleDistributionItem::childParameterNames() const
+QString ParticleDistributionItem::translateParameterNameToGUI(const QString& domainName)
 {
-    if(getItems(T_PARTICLES).size() == 0)
-        return {};
+    if(auto child = childParticle())
+        return ParameterTreeUtils::domainNameToParameterName(domainName, child);
 
-    Q_ASSERT(getItems(T_PARTICLES).size() == 1);
-
-    return ParameterTreeUtils::parameterTreeNames(getItems(T_PARTICLES).front());
+    return {};
 }
 
-QString ParticleDistributionItem::translateParameterNameToGUI(const QString& domainName)
+const SessionItem* ParticleDistributionItem::childParticle() const
 {
-    if(getItems(T_PARTICLES).size() == 0)
-        return {};
-
-    Q_ASSERT(getItems(T_PARTICLES).size() > 0);
+    if (getItems(T_PARTICLES).size() == 0)
+        return nullptr;
 
-    return ParameterTreeUtils::domainNameToParameterName(domainName, getItems(T_PARTICLES).front());
+    Q_ASSERT(getItems(T_PARTICLES).size() == 1);
+    return getItems(T_PARTICLES).front();
 }
-
diff --git a/GUI/coregui/Models/ParticleDistributionItem.h b/GUI/coregui/Models/ParticleDistributionItem.h
index 811a115d1a9..d185d83e2d0 100644
--- a/GUI/coregui/Models/ParticleDistributionItem.h
+++ b/GUI/coregui/Models/ParticleDistributionItem.h
@@ -35,12 +35,10 @@ public:
 
     void setDomainCacheName(const QString& name);
 
-    bool cacheContainsGUIValue() const;
-    QString domainCacheName() const;
-
 private:
     QStringList childParameterNames() const;
     QString translateParameterNameToGUI(const QString& domainName);
+    const SessionItem* childParticle() const;
     QString m_domain_cache_name;
 };
 
diff --git a/GUI/coregui/Models/TransformFromDomain.cpp b/GUI/coregui/Models/TransformFromDomain.cpp
index 1d43ee197a4..7fcfccb54ad 100644
--- a/GUI/coregui/Models/TransformFromDomain.cpp
+++ b/GUI/coregui/Models/TransformFromDomain.cpp
@@ -157,24 +157,18 @@ void TransformFromDomain::setItemFromSample(SessionItem* item, const LayerRoughn
 void TransformFromDomain::setItemFromSample(SessionItem* item,
                                             const ParticleDistribution* sample)
 {
-    item->setItemValue(ParticleItem::P_ABUNDANCE, sample->getAbundance());
+    ParticleDistributionItem *distItem = dynamic_cast<ParticleDistributionItem*>(item);
+    Q_ASSERT(distItem);
+
+    distItem->setItemValue(ParticleItem::P_ABUNDANCE, sample->getAbundance());
 
     ParameterDistribution par_distr = sample->getParameterDistribution();
     QString main_distr_par_name = QString::fromStdString(par_distr.getMainParameterName());
-    ComboProperty combo_property
-        = item->getItemValue(ParticleDistributionItem::P_DISTRIBUTED_PARAMETER)
-              .value<ComboProperty>();
-    combo_property.setCachedValue(main_distr_par_name);
-    combo_property.setCacheContainsGUIFlag(false);
 
-    ParticleDistributionItem *distItem = dynamic_cast<ParticleDistributionItem*>(item);
     distItem->setDomainCacheName(main_distr_par_name);
 
-    item->setItemValue(ParticleDistributionItem::P_DISTRIBUTED_PARAMETER,
-                                combo_property.getVariant());
-
     QString group_name = ParticleDistributionItem::P_DISTRIBUTION;
-    setDistribution(item, par_distr, group_name);
+    setDistribution(distItem, par_distr, group_name);
 }
 
 //! Returns true if given roughness is non-zero roughness
diff --git a/Tests/UnitTests/GUI/TestComboProperty.h b/Tests/UnitTests/GUI/TestComboProperty.h
index fa5b156368e..0b6bdd3368c 100644
--- a/Tests/UnitTests/GUI/TestComboProperty.h
+++ b/Tests/UnitTests/GUI/TestComboProperty.h
@@ -7,6 +7,7 @@ class TestComboProperty : public QObject {
 private slots:
     void test_ComboEquality();
     void test_VariantEquality();
+    void test_setValue();
 };
 
 inline void TestComboProperty::test_ComboEquality()
@@ -50,3 +51,12 @@ inline void TestComboProperty::test_VariantEquality()
     c1.setValue("a2");
     QVERIFY(c1.getVariant() == c2.getVariant());
 }
+
+inline void TestComboProperty::test_setValue()
+{
+    QStringList expectedValues = QStringList() << "a1" << "a2";
+    ComboProperty combo = ComboProperty() << expectedValues;
+
+    QCOMPARE(combo.getValue(), QString("a1"));
+}
+
-- 
GitLab