Merge branch 'refactor_model2' into 'develop'

Refactor model2: improve interface of IntensityDataItem

See merge request !66

GUI/coregui/Models/IntensityDataItem.cpp

@@ -46,7 +46,7 @@ const QString x_axis_default_name = "X [nbins]";
 const QString y_axis_default_name = "Y [nbins]";
 
 const QString IntensityDataItem::P_TITLE = "Title";
-const QString IntensityDataItem::P_PROJECTIONS_FLAG = "Projections";
+const QString IntensityDataItem::P_PROJECTIONS = "Projections";
 const QString IntensityDataItem::P_IS_INTERPOLATED = "Interpolation";
 const QString IntensityDataItem::P_GRADIENT = "Gradient";
 const QString IntensityDataItem::P_XAXIS = "x-axis";
@@ -59,7 +59,7 @@ IntensityDataItem::IntensityDataItem() : DataItem("IntensityData")
 {
     addProperty(P_TITLE, QString())->setVisible(false);
 
-    addProperty(P_PROJECTIONS_FLAG, false)->setVisible(false);
+    addProperty(P_PROJECTIONS, false)->setVisible(false);
     addProperty(P_IS_INTERPOLATED, true);
     addProperty(P_GRADIENT, gradientCombo().variant());
 
@@ -160,10 +160,19 @@ double IntensityDataItem::getUpperZ() const
     return zAxisItem()->upperBound();
 }
 
-QString IntensityDataItem::getGradient() const
+QString IntensityDataItem::getGradientValue() const
 {
-    ComboProperty combo_property = getItemValue(P_GRADIENT).value<ComboProperty>();
-    return combo_property.getValue();
+    return getGradient().getValue();
+}
+
+ComboProperty IntensityDataItem::getGradient() const
+{
+    return getItemValue(P_GRADIENT).value<ComboProperty>();
+}
+
+void IntensityDataItem::setGradient(const ComboProperty& gradient)
+{
+    setItemValue(P_GRADIENT, gradient.variant());
 }
 
 bool IntensityDataItem::isLogz() const
@@ -246,6 +255,16 @@ void IntensityDataItem::reset(ImportDataInfo data)
     converter.convertFromNbins(this);
 }
 
+bool IntensityDataItem::isInterpolatedPropertyName(const QString& name)
+{
+    return name == P_IS_INTERPOLATED;
+}
+
+bool IntensityDataItem::isGradientPropertyName(const QString& name)
+{
+    return name == P_GRADIENT;
+}
+
 void IntensityDataItem::setLowerX(double value)
 {
     xAxisItem()->setLowerBound(value);
@@ -436,3 +455,8 @@ bool IntensityDataItem::hasProjections() const
 {
     return (projectionContainerItem() && projectionContainerItem()->hasChildren());
 }
+
+SessionItem* IntensityDataItem::projectionsItem() const
+{
+    return getItem(P_PROJECTIONS);
+}

GUI/coregui/Models/IntensityDataItem.h

@@ -23,11 +23,13 @@ class MaskContainerItem;
 class ProjectionContainerItem;
 
 class BA_CORE_API_ IntensityDataItem : public DataItem {
-public:
-    static const QString P_PROJECTIONS_FLAG;
+private:
+    static const QString P_PROJECTIONS;
     static const QString P_TITLE;
     static const QString P_IS_INTERPOLATED;
     static const QString P_GRADIENT;
+
+public:
     static const QString P_XAXIS;
     static const QString P_YAXIS;
     static const QString P_ZAXIS;
@@ -63,7 +65,10 @@ public:
     double getLowerZ() const;
     double getUpperZ() const;
 
-    QString getGradient() const;
+    QString getGradientValue() const;
+    ComboProperty getGradient() const;
+    void setGradient(const ComboProperty& gradient);
+
     bool isLogz() const;
     bool isInterpolated() const;
     QString getXaxisTitle() const;
@@ -93,7 +98,8 @@ public:
 
     bool hasMasks() const;
     bool hasProjections() const;
-
+    SessionItem* projectionsItem() const;
+    
     void setXaxisTitle(const QString& title) override;
     void setYaxisTitle(const QString& title) override;
     void setAxesRangeToData() override;
@@ -103,6 +109,9 @@ public:
     //! Returns data to the state defined by user (imported) data.
     void reset(ImportDataInfo data) override;
 
+    static bool isInterpolatedPropertyName(const QString& name);
+    static bool isGradientPropertyName(const QString& name);
+                                            
 public slots:
     void setLowerX(double value);
     void setUpperX(double value);

GUI/coregui/Views/InstrumentWidgets/DetectorMaskDelegate.cpp

@@ -57,8 +57,8 @@ void DetectorMaskDelegate::createIntensityDataItem()
 {
     m_tempIntensityDataModel->clear();
     m_intensityItem = m_tempIntensityDataModel->insertItem<IntensityDataItem>();
-    m_intensityItem->getItem(IntensityDataItem::P_PROJECTIONS_FLAG)->setEnabled(false);
-    m_intensityItem->setItemValue(IntensityDataItem::P_IS_INTERPOLATED, false);
+    m_intensityItem->projectionsItem()->setEnabled(false);
+    m_intensityItem->setInterpolated(false);
 
     auto zAxisItem = m_intensityItem->zAxisItem();
     zAxisItem->setItemValue(BasicAxisItem::P_IS_VISIBLE, false);

GUI/coregui/Views/IntensityDataWidgets/ColorMap.cpp

@@ -109,10 +109,10 @@ void ColorMap::onPropertyChanged(const QString& property_name)
     if (m_block_update)
         return;
 
-    if (property_name == IntensityDataItem::P_GRADIENT) {
+    if (IntensityDataItem::isGradientPropertyName(property_name)) {
         m_colorMap->setGradient(ColorMapUtils::itemGradient(intensityItem()));
         replot();
-    } else if (property_name == IntensityDataItem::P_IS_INTERPOLATED) {
+    } else if (IntensityDataItem::isInterpolatedPropertyName(property_name)) {
         m_colorMap->setInterpolate(intensityItem()->isInterpolated());
         replot();
     } else if (DataItem::isAxesUnitsPropertyName(property_name)) {

GUI/coregui/Views/IntensityDataWidgets/ColorMapUtils.cpp

@@ -75,7 +75,7 @@ QCPColorGradient ColorMapUtils::getGradient(const QString& gradientName)
 
 QCPColorGradient ColorMapUtils::itemGradient(const IntensityDataItem* item)
 {
-    return getGradient(item->getGradient());
+    return getGradient(item->getGradientValue());
 }
 
 QCPRange ColorMapUtils::itemXrange(const IntensityDataItem* item)

GUI/coregui/Views/IntensityDataWidgets/IntensityDataCanvas.cpp

@@ -28,18 +28,12 @@
 
 namespace {
 
-QString group_name()
-{
-    return "IntensityDataCanvas/";
-}
-QString gradient_setting_name()
-{
-    return group_name() + IntensityDataItem::P_GRADIENT;
-}
-QString interpolation_setting_name()
-{
-    return group_name() + IntensityDataItem::P_IS_INTERPOLATED;
-}
+const QString SettingsGroup = "IntensityDataCanvas/";
+    
+const QString SettingsKeyGradient = SettingsGroup + "Gradient";
+
+const QString SettingsKeyInterpolation = SettingsGroup + "Interpolation";
+
 } // namespace
 
 IntensityDataCanvas::IntensityDataCanvas(QWidget* parent)
@@ -139,12 +133,14 @@ void IntensityDataCanvas::initActions()
 
 void IntensityDataCanvas::onPropertyChanged(const QString& name)
 {
-    if (name == IntensityDataItem::P_GRADIENT) {
+    if (IntensityDataItem::isGradientPropertyName(name)) {
         QSettings settings;
-        settings.setValue(gradient_setting_name(), intensityDataItem()->getGradient());
-    } else if (name == IntensityDataItem::P_IS_INTERPOLATED) {
+        settings.setValue(SettingsKeyGradient,
+                          intensityDataItem()->getGradientValue());
+    } else if (IntensityDataItem::isInterpolatedPropertyName(name)) {
         QSettings settings;
-        settings.setValue(interpolation_setting_name(), intensityDataItem()->isInterpolated());
+        settings.setValue(SettingsKeyInterpolation,
+                          intensityDataItem()->isInterpolated());
     }
 }
 
@@ -154,18 +150,17 @@ void IntensityDataCanvas::applyPersistentSettings()
 {
     QSettings settings;
 
-    if (settings.contains(gradient_setting_name())) {
-        ComboProperty combo =
-            intensityDataItem()->getItemValue(IntensityDataItem::P_GRADIENT).value<ComboProperty>();
-        QString persistentGradient = settings.value(gradient_setting_name()).toString();
+    if (settings.contains(SettingsKeyGradient)) {
+        ComboProperty combo = intensityDataItem()->getGradient();
+        QString persistentGradient = settings.value(SettingsKeyGradient).toString();
         if (combo.getValue() != persistentGradient) {
-            combo.setValue(settings.value(gradient_setting_name()).toString());
-            intensityDataItem()->setItemValue(IntensityDataItem::P_GRADIENT, combo.variant());
+            combo.setValue(persistentGradient);
+            intensityDataItem()->setGradient(combo);
         }
     }
 
-    if (settings.contains(interpolation_setting_name())) {
-        bool value = settings.value(interpolation_setting_name()).toBool();
-        intensityDataItem()->setItemValue(IntensityDataItem::P_IS_INTERPOLATED, value);
+    if (settings.contains(SettingsKeyInterpolation)) {
+        bool value = settings.value(SettingsKeyInterpolation).toBool();
+        intensityDataItem()->setInterpolated(value);
     }
 }

GUI/coregui/Views/IntensityDataWidgets/ProjectionsPlot.cpp

@@ -244,7 +244,7 @@ void ProjectionsPlot::clearProjection(SessionItem* item)
 
 void ProjectionsPlot::onIntensityItemPropertyChanged(const QString& propertyName)
 {
-    if (propertyName == IntensityDataItem::P_IS_INTERPOLATED) {
+    if (IntensityDataItem::isInterpolatedPropertyName(propertyName)) {
         setInterpolate(intensityItem()->isInterpolated());
         replot();
     }

GUI/coregui/Views/MaskWidgets/MaskResultsPresenter.cpp

@@ -59,7 +59,7 @@ void MaskResultsPresenter::setShowMaskMode()
     if (OutputData<double>* maskedData = createMaskPresentation()) {
         backup_data();
         m_intensityDataItem->setOutputData(maskedData);
-        m_intensityDataItem->setItemValue(IntensityDataItem::P_IS_INTERPOLATED, false);
+        m_intensityDataItem->setInterpolated(false);
     } else {
         m_dataBackup.reset();
     }
@@ -71,15 +71,13 @@ void MaskResultsPresenter::setOriginalMode()
 {
     if (m_dataBackup) {
         m_intensityDataItem->setOutputData(m_dataBackup->clone());
-        m_intensityDataItem->setItemValue(IntensityDataItem::P_IS_INTERPOLATED,
-                                          m_interpolation_flag_backup);
+        m_intensityDataItem->setInterpolated(m_interpolation_flag_backup);
     }
 }
 
 void MaskResultsPresenter::backup_data()
 {
-    m_interpolation_flag_backup =
-        m_intensityDataItem->getItemValue(IntensityDataItem::P_IS_INTERPOLATED).toBool();
+    m_interpolation_flag_backup = m_intensityDataItem->isInterpolated();
     m_dataBackup.reset(m_intensityDataItem->getOutputData()->clone());
 }
 

Tests/UnitTests/GUI/TestPropertyRepeater.cpp

@@ -84,8 +84,8 @@ TEST_F(TestPropertyRepeater, test_repeatAll)
 {
     SessionModel model("test");
 
-    auto item1 = createData(model);
-    auto item2 = createData(model);
+    IntensityDataItem* item1 = createData(model);
+    IntensityDataItem* item2 = createData(model);
 
     item1->xAxisItem()->setItemValue(BasicAxisItem::P_MAX_DEG, 2.0);
     item2->xAxisItem()->setItemValue(BasicAxisItem::P_MAX_DEG, 3.0);
@@ -96,8 +96,8 @@ TEST_F(TestPropertyRepeater, test_repeatAll)
     repeater.addItem(item2);
 
     // adding items to the repeater do not change values
-    EXPECT_EQ(item1->getItemValue(IntensityDataItem::P_IS_INTERPOLATED).toBool(), true);
-    EXPECT_EQ(item2->getItemValue(IntensityDataItem::P_IS_INTERPOLATED).toBool(), true);
+    EXPECT_TRUE(item1->isInterpolated());
+    EXPECT_TRUE(item2->isInterpolated());
     EXPECT_EQ(item1->getUpperX(), 2.0);
     EXPECT_EQ(item2->getUpperX(), 3.0);
 
@@ -106,9 +106,9 @@ TEST_F(TestPropertyRepeater, test_repeatAll)
     EXPECT_EQ(item1->getUpperX(), 4.0);
     EXPECT_EQ(item2->getUpperX(), 4.0);
 
-    item1->setItemValue(IntensityDataItem::P_IS_INTERPOLATED, false);
-    EXPECT_EQ(item1->getItemValue(IntensityDataItem::P_IS_INTERPOLATED).toBool(), false);
-    EXPECT_EQ(item2->getItemValue(IntensityDataItem::P_IS_INTERPOLATED).toBool(), false);
+    item1->setInterpolated(false);
+    EXPECT_FALSE(item1->isInterpolated());
+    EXPECT_FALSE(item2->isInterpolated());
 
     // clearing repeater will stop update
     repeater.clear();