From 5083b9a85a1a3e52fea1a6f7879b86c3ac746efc Mon Sep 17 00:00:00 2001
From: Matthias Puchner <github@mpuchner.de>
Date: Thu, 22 Jul 2021 15:42:47 +0200
Subject: [PATCH] split computation method setting in two methods; use string
 constants

---
 GUI/Models/SimulationOptionsItem.cpp    | 63 +++++++++++++------------
 GUI/Models/SimulationOptionsItem.h      |  3 +-
 Tests/UnitTests/GUI/TestMapperCases.cpp |  2 +-
 3 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/GUI/Models/SimulationOptionsItem.cpp b/GUI/Models/SimulationOptionsItem.cpp
index dd052aacb17..40b3f9960f7 100644
--- a/GUI/Models/SimulationOptionsItem.cpp
+++ b/GUI/Models/SimulationOptionsItem.cpp
@@ -36,6 +36,14 @@ const QString tooltip_ambientmaterial =
 const QString tooltip_specularpeak =
     "Defines if the specular peak should be included in the simulation result";
 
+const QString Immediately = "Immediately";
+const QString InBackground = "In background";
+const QString Analytical = "Analytical";
+const QString MonteCarlo = "Monte-Carlo Integration";
+const QString AmbientLayerMaterial = "Ambient Layer Material";
+const QString AverageLayerMaterial = "Average Layer Material";
+const QString Yes = "Yes";
+const QString No = "No";
 } // namespace
 
 const QString SimulationOptionsItem::P_RUN_POLICY = "Run Policy";
@@ -61,21 +69,18 @@ SimulationOptionsItem::SimulationOptionsItem() : SessionItem(M_TYPE)
     addProperty(P_NTHREADS, nthreads.variant())->setToolTip(tooltip_nthreads);
 
     ComboProperty computationMethod;
-    computationMethod << "Analytical"
-                      << "Monte-Carlo Integration";
+    computationMethod << Analytical << MonteCarlo;
     addProperty(P_COMPUTATION_METHOD, computationMethod.variant())->setToolTip(tooltip_computation);
 
     addProperty(P_MC_POINTS, 100)->setEnabled(false);
 
     ComboProperty averageLayerMaterials;
-    averageLayerMaterials << "Ambient Layer Material"
-                          << "Average Layer Material";
+    averageLayerMaterials << AmbientLayerMaterial << AverageLayerMaterial;
     addProperty(P_FRESNEL_MATERIAL_METHOD, averageLayerMaterials.variant())
         ->setToolTip(tooltip_ambientmaterial);
 
     ComboProperty includeSpecularPeak;
-    includeSpecularPeak << "No"
-                        << "Yes";
+    includeSpecularPeak << No << Yes;
     addProperty(P_INCLUDE_SPECULAR_PEAK, includeSpecularPeak.variant())
         ->setToolTip(tooltip_specularpeak);
 
@@ -83,17 +88,14 @@ SimulationOptionsItem::SimulationOptionsItem() : SessionItem(M_TYPE)
         if (name == P_COMPUTATION_METHOD && isTag(P_MC_POINTS)) {
             ComboProperty combo = getItemValue(P_COMPUTATION_METHOD).value<ComboProperty>();
 
-            if (combo.getValue() == "Analytical") {
+            if (combo.getValue() == Analytical)
                 getItem(P_MC_POINTS)->setEnabled(false);
-
-            } else {
+            else
                 getItem(P_MC_POINTS)->setEnabled(true);
-            }
-        } else if (name == P_NTHREADS) {
+        } else if (name == P_NTHREADS)
             updateComboItem(P_NTHREADS, getCPUUsageOptions());
-        } else if (name == P_RUN_POLICY) {
+        else if (name == P_RUN_POLICY)
             updateComboItem(P_RUN_POLICY, getRunPolicyNames());
-        }
     });
 }
 
@@ -117,17 +119,17 @@ void SimulationOptionsItem::setNumberOfThreads(int number)
 
 bool SimulationOptionsItem::runImmediately() const
 {
-    return runPolicy() == "Immediately";
+    return runPolicy() == Immediately;
 }
 
 bool SimulationOptionsItem::runInBackground() const
 {
-    return runPolicy() == "In background";
+    return !runImmediately();
 }
 
 void SimulationOptionsItem::setRunImmediately(bool runImmediately)
 {
-    setRunPolicy(runImmediately ? "Immediately" : "In background");
+    setRunPolicy(runImmediately ? Immediately : InBackground);
 }
 
 void SimulationOptionsItem::setRunPolicy(const QString& policy)
@@ -137,19 +139,20 @@ void SimulationOptionsItem::setRunPolicy(const QString& policy)
     setItemValue(P_RUN_POLICY, combo.variant());
 }
 
-void SimulationOptionsItem::setMonteCarloIntegration(bool useMonteCarlo, int numberOfPoints)
+void SimulationOptionsItem::setUseMonteCarloIntegration(int numberOfPoints)
 {
-    if (useMonteCarlo) {
-        setComputationMethod("Monte-Carlo Integration");
-        setNumberOfMonteCarloPoints(numberOfPoints);
-    } else {
-        setComputationMethod("Analytical");
-    }
+    setComputationMethod(MonteCarlo);
+    setNumberOfMonteCarloPoints(numberOfPoints);
+}
+
+void SimulationOptionsItem::setUseAnalytical()
+{
+    setComputationMethod(Analytical);
 }
 
 bool SimulationOptionsItem::useMonteCarloIntegration() const
 {
-    return getComputationMethod().startsWith("Mon");
+    return getComputationMethod() == MonteCarlo;
 }
 
 void SimulationOptionsItem::setComputationMethod(const QString& name)
@@ -177,13 +180,12 @@ void SimulationOptionsItem::setNumberOfMonteCarloPoints(int npoints)
 
 void SimulationOptionsItem::setUseAverageMaterials(bool useAverageMaterials)
 {
-    setFresnelMaterialMethod(useAverageMaterials ? "Average Layer Material"
-                                                 : "Ambient Layer Material");
+    setFresnelMaterialMethod(useAverageMaterials ? AverageLayerMaterial : AmbientLayerMaterial);
 }
 
 bool SimulationOptionsItem::useAverageMaterials() const
 {
-    return getFresnelMaterialMethod().startsWith("Aver");
+    return getFresnelMaterialMethod() == AverageLayerMaterial;
 }
 
 void SimulationOptionsItem::setFresnelMaterialMethod(const QString& name)
@@ -202,14 +204,14 @@ QString SimulationOptionsItem::getFresnelMaterialMethod() const
 void SimulationOptionsItem::setIncludeSpecularPeak(bool includeSpecularPeak)
 {
     ComboProperty combo = getItemValue(P_INCLUDE_SPECULAR_PEAK).value<ComboProperty>();
-    combo.setValue(includeSpecularPeak ? "Yes" : "No");
+    combo.setValue(includeSpecularPeak ? Yes : No);
     setItemValue(P_INCLUDE_SPECULAR_PEAK, combo.variant());
 }
 
 bool SimulationOptionsItem::includeSpecularPeak() const
 {
     ComboProperty combo = getItemValue(P_INCLUDE_SPECULAR_PEAK).value<ComboProperty>();
-    return combo.getValue() == "Yes";
+    return combo.getValue() == Yes;
 }
 
 SessionItem* SimulationOptionsItem::monteCarloPointsItem() const
@@ -247,8 +249,7 @@ QStringList SimulationOptionsItem::getCPUUsageOptions()
 QStringList SimulationOptionsItem::getRunPolicyNames()
 {
     QStringList result;
-    result << "Immediately"
-           << "In background";
+    result << Immediately << InBackground;
     return result;
 }
 
diff --git a/GUI/Models/SimulationOptionsItem.h b/GUI/Models/SimulationOptionsItem.h
index 8bf67adc42a..642c28863cc 100644
--- a/GUI/Models/SimulationOptionsItem.h
+++ b/GUI/Models/SimulationOptionsItem.h
@@ -43,7 +43,8 @@ public:
     bool runImmediately() const;
     bool runInBackground() const;
 
-    void setMonteCarloIntegration(bool useMonteCarlo, int numberOfPoints = 50);
+    void setUseMonteCarloIntegration(int numberOfPoints);
+    void setUseAnalytical();
     bool useMonteCarloIntegration() const;
 
     void setNumberOfMonteCarloPoints(int npoints);
diff --git a/Tests/UnitTests/GUI/TestMapperCases.cpp b/Tests/UnitTests/GUI/TestMapperCases.cpp
index d103be6f2e7..d0d1757c1ef 100644
--- a/Tests/UnitTests/GUI/TestMapperCases.cpp
+++ b/Tests/UnitTests/GUI/TestMapperCases.cpp
@@ -38,6 +38,6 @@ TEST_F(TestMapperCases, test_SimulationOptionsComputationToggle)
     EXPECT_EQ(item->useMonteCarloIntegration(), false);
     EXPECT_FALSE(item->monteCarloPointsItem()->isEnabled());
 
-    item->setMonteCarloIntegration(true);
+    item->setUseMonteCarloIntegration(100);
     EXPECT_TRUE(item->monteCarloPointsItem()->isEnabled());
 }
-- 
GitLab