Merge pull request gpospelov#332 from gpospelov/Data2DItemRefactoring

Data2DItem refactoring
tvgugylu · Feb 12, 2021 · 99cb43f · 99cb43f
2 parents b3966c4 + 542c462
commit 99cb43f
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 35 deletions.
diff --git a/examples/plotcolormap/plotcolormapcore/colormapmodel.cpp b/examples/plotcolormap/plotcolormapcore/colormapmodel.cpp
@@ -43,20 +43,20 @@ namespace PlotColorMap {
 
 ColorMapModel::ColorMapModel() : SessionModel("ColorMapModel")
 {
-    init_model();
+    populateModel();
 }
 
 //! Updates data.
 
-void ColorMapModel::update_data(double scale)
+void ColorMapModel::updateData(double scale)
 {
-    auto data_item = data_container()->item<Data2DItem>(ContainerItem::T_ITEMS);
+    auto data_item = dataContainer()->item<Data2DItem>(ContainerItem::T_ITEMS);
     fill_data(data_item, scale);
 }
 
-void ColorMapModel::add_colormap()
+void ColorMapModel::addColormap()
 {
-    auto data_item = insertItem<Data2DItem>(data_container());
+    auto data_item = insertItem<Data2DItem>(dataContainer());
     data_item->setAxes(FixedBinAxisItem::create(nbinsx, -5.0, 5.0),
                        FixedBinAxisItem::create(nbinsy, 0.0, 5.0));
     fill_data(data_item);
@@ -66,17 +66,17 @@ void ColorMapModel::add_colormap()
     colormap_item->setDataItem(data_item);
 }
 
-ContainerItem* ColorMapModel::data_container()
+ContainerItem* ColorMapModel::dataContainer()
 {
     return topItem<ContainerItem>();
 }
 
-void ColorMapModel::init_model()
+void ColorMapModel::populateModel()
 {
     auto container = insertItem<ContainerItem>();
     container->setDisplayName("Data container");
 
-    add_colormap();
+    addColormap();
 }
 
 } // namespace PlotColorMap
diff --git a/examples/plotcolormap/plotcolormapcore/colormapmodel.h b/examples/plotcolormap/plotcolormapcore/colormapmodel.h
@@ -24,12 +24,12 @@ class ColorMapModel : public ModelView::SessionModel {
 public:
     ColorMapModel();
 
-    void update_data(double scale);
+    void updateData(double scale);
 
 private:
-    ModelView::ContainerItem* data_container();
-    void init_model();
-    void add_colormap();
+    ModelView::ContainerItem* dataContainer();
+    void populateModel();
+    void addColormap();
 };
 
 } // namespace PlotColorMap

diff --git a/examples/plotcolormap/plotcolormapcore/colormappropertywidget.cpp b/examples/plotcolormap/plotcolormapcore/colormappropertywidget.cpp
@@ -20,14 +20,14 @@ using namespace ModelView;
 namespace PlotColorMap {
 
 ColorMapPropertyWidget::ColorMapPropertyWidget(ColorMapModel* model, QWidget* parent)
-    : QWidget(parent), m_slider(new QSlider), m_treeView(new ItemsTreeView), m_model(model)
+    : QWidget(parent), m_slider(new QSlider), m_tree_view(new ItemsTreeView), m_model(model)
 {
     auto layout = new QVBoxLayout;
     layout->addWidget(m_slider);
-    layout->addWidget(m_treeView);
+    layout->addWidget(m_tree_view);
     setLayout(layout);
     setModel(model);
-    setup_slider();
+    setupSlider();
 }
 
 void ColorMapPropertyWidget::setModel(ColorMapModel* model)
@@ -37,18 +37,18 @@ void ColorMapPropertyWidget::setModel(ColorMapModel* model)
 
     m_model = model;
 
-    m_treeView->setViewModel(Factory::CreateDefaultViewModel(model));
+    m_tree_view->setViewModel(Factory::CreateDefaultViewModel(model));
 }
 
 //! Slider to regenerate the data in the model.
 
-void ColorMapPropertyWidget::setup_slider()
+void ColorMapPropertyWidget::setupSlider()
 {
     m_slider->setOrientation(Qt::Horizontal);
     m_slider->setRange(0, 100);
     m_slider->setValue(50.0);
 
-    auto on_value_changed = [this](int value) { m_model->update_data(value / 10.0); };
+    auto on_value_changed = [this](int value) { m_model->updateData(value / 10.0); };
     connect(m_slider, &QSlider::valueChanged, on_value_changed);
 }
 

diff --git a/examples/plotcolormap/plotcolormapcore/colormappropertywidget.h b/examples/plotcolormap/plotcolormapcore/colormappropertywidget.h
@@ -36,9 +36,9 @@ class ColorMapPropertyWidget : public QWidget {
     void setModel(ColorMapModel* model);
 
 private:
-    void setup_slider();
+    void setupSlider();
     QSlider* m_slider{nullptr};
-    ModelView::ItemsTreeView* m_treeView{nullptr};
+    ModelView::ItemsTreeView* m_tree_view{nullptr};
     ColorMapModel* m_model{nullptr};
 };
 

diff --git a/source/libmvvm_model/mvvm/standarditems/data1ditem.cpp b/source/libmvvm_model/mvvm/standarditems/data1ditem.cpp
@@ -31,7 +31,6 @@ Data1DItem::Data1DItem() : CompoundItem(Constants::Data1DItemType)
     registerTag(
         TagInfo(T_AXIS, 0, 1, {Constants::FixedBinAxisItemType, Constants::PointwiseAxisItemType}),
         true);
-    setValues(std::vector<double>());
 }
 
 //! Sets axis. Bin content will be set to zero.

diff --git a/source/libmvvm_model/mvvm/standarditems/data2ditem.cpp b/source/libmvvm_model/mvvm/standarditems/data2ditem.cpp
@@ -25,10 +25,11 @@ size_t total_bin_count(Data2DItem* item)
 
 Data2DItem::Data2DItem() : CompoundItem(Constants::Data2DItemType)
 {
+    // prevent editing in widgets, since there is no corresponding editor
+    addProperty(P_VALUES, std::vector<double>())->setDisplayName("Values")->setEditable(false);
+
     registerTag(TagInfo(T_XAXIS, 0, 1, {Constants::FixedBinAxisItemType}));
     registerTag(TagInfo(T_YAXIS, 0, 1, {Constants::FixedBinAxisItemType}));
-    setContent(std::vector<double>()); // prevent editing in widgets, since there is no
-                                       // corresponding editor
 }
 
 //! Sets axes and put data points to zero.
@@ -59,15 +60,14 @@ void Data2DItem::setContent(const std::vector<double>& data)
 {
     if (total_bin_count(this) != data.size())
         throw std::runtime_error("Data1DItem::setContent() -> Data doesn't match size of axis");
-
-    setData(data);
+    setProperty(P_VALUES, data);
 }
 
-//! Returns 2d vector representing 2d data.
+//! Returns 1d buffer representing 2d data.
 
 std::vector<double> Data2DItem::content() const
 {
-    return data<std::vector<double>>();
+    return property<std::vector<double>>(P_VALUES);
 }
 
 //! Insert axis under given tag. Previous axis will be deleted and data points invalidated.

diff --git a/source/libmvvm_model/mvvm/standarditems/data2ditem.h b/source/libmvvm_model/mvvm/standarditems/data2ditem.h
@@ -23,6 +23,7 @@ class BinnedAxisItem;
 
 class MVVM_MODEL_EXPORT Data2DItem : public CompoundItem {
 public:
+    static inline const std::string P_VALUES = "P_VALUES";
     static inline const std::string T_XAXIS = "T_XAXIS";
     static inline const std::string T_YAXIS = "T_YAXIS";
 

diff --git a/source/libmvvm_view/mvvm/plotting/data2dplotcontroller.cpp b/source/libmvvm_view/mvvm/plotting/data2dplotcontroller.cpp
@@ -76,8 +76,11 @@ Data2DPlotController::~Data2DPlotController() = default;
 
 void Data2DPlotController::subscribe()
 {
-    auto on_data_change = [this](SessionItem*, int) { p_impl->update_data_points(); };
-    setOnDataChange(on_data_change);
+    auto on_property_change = [this](SessionItem*, const std::string& name) {
+        if (name == Data2DItem::P_VALUES)
+            p_impl->update_data_points();
+    };
+    setOnPropertyChange(on_property_change);
 
     p_impl->update_data_points();
 }

diff --git a/tests/testmodel/data2ditem.test.cpp b/tests/testmodel/data2ditem.test.cpp
@@ -32,8 +32,7 @@ TEST_F(Data2DItemTest, initialState)
     EXPECT_EQ(item.xAxis(), nullptr);
     EXPECT_EQ(item.yAxis(), nullptr);
     EXPECT_EQ(item.content(), std::vector<double>());
-    EXPECT_TRUE(item.hasData());
-    EXPECT_TRUE(item.data<std::vector<double>>().empty());
+    EXPECT_FALSE(item.hasData());
 }
 
 //! Checking the method ::setAxis.
@@ -83,8 +82,8 @@ TEST_F(Data2DItemTest, checkSignalsOnAxisChange)
 
     MockWidgetForItem widget(item);
 
-    EXPECT_CALL(widget, onDataChange(item, ItemDataRole::DATA)).Times(1); // values should change
-    EXPECT_CALL(widget, onPropertyChange(_, _)).Times(0);
+    EXPECT_CALL(widget, onDataChange(item, ItemDataRole::DATA)).Times(0);
+    EXPECT_CALL(widget, onPropertyChange(_, Data2DItem::P_VALUES)).Times(1);
     EXPECT_CALL(widget, onChildPropertyChange(_, _)).Times(0);
     EXPECT_CALL(widget, onItemInserted(item, _)).Times(2);
     EXPECT_CALL(widget, onAboutToRemoveItem(_, _)).Times(0);
@@ -103,8 +102,8 @@ TEST_F(Data2DItemTest, checkSignalsOnContentChange)
 
     MockWidgetForItem widget(item);
 
-    EXPECT_CALL(widget, onDataChange(item, ItemDataRole::DATA)).Times(1); // values should change
-    EXPECT_CALL(widget, onPropertyChange(_, _)).Times(0);
+    EXPECT_CALL(widget, onDataChange(item, ItemDataRole::DATA)).Times(0);
+    EXPECT_CALL(widget, onPropertyChange(_, Data2DItem::P_VALUES)).Times(1);
     EXPECT_CALL(widget, onChildPropertyChange(_, _)).Times(0);
     EXPECT_CALL(widget, onItemInserted(_, _)).Times(0);
     EXPECT_CALL(widget, onAboutToRemoveItem(_, _)).Times(0);