From 7d5bf210f984894de00a771305cb552af44d43f8 Mon Sep 17 00:00:00 2001
From: Matthias Puchner <github@mpuchner.de>
Date: Thu, 2 Dec 2021 16:28:45 +0100
Subject: [PATCH] reduce SessionItem usage for MultiLayerItem

---
 GUI/Model/From/GUIDomainSampleVisitor.cpp  |  2 +-
 GUI/Model/From/SampleListModel.cpp         | 10 +++++-----
 GUI/Model/Job/JobModel.cpp                 |  2 +-
 GUI/Model/Job/JobModelFunctions.cpp        |  2 +-
 GUI/Model/Sample/MultiLayerItem.cpp        | 10 ++++++++++
 GUI/Model/Sample/MultiLayerItem.h          |  8 ++++++--
 GUI/Model/To/DomainObjectBuilder.cpp       | 10 +++++-----
 GUI/View/SampleDesigner/SampleListView.cpp |  2 +-
 GUI/View/Toplevel/SimulationView.cpp       | 12 +++++++++++-
 Tests/Unit/GUI/TestMapperForItem.cpp       |  2 +-
 Tests/Unit/GUI/TestModelUtils.cpp          |  4 ++--
 Tests/Unit/GUI/TestMultiLayerItem.cpp      | 22 +++++++++++-----------
 Tests/Unit/GUI/TestParaCrystalItems.cpp    |  2 +-
 Tests/Unit/GUI/TestSessionModel.cpp        | 14 +++++++-------
 14 files changed, 63 insertions(+), 39 deletions(-)

diff --git a/GUI/Model/From/GUIDomainSampleVisitor.cpp b/GUI/Model/From/GUIDomainSampleVisitor.cpp
index f5de5e70078..3cb299f1254 100644
--- a/GUI/Model/From/GUIDomainSampleVisitor.cpp
+++ b/GUI/Model/From/GUIDomainSampleVisitor.cpp
@@ -133,7 +133,7 @@ void GUIDomainSampleVisitor::visit(const Layer* node)
 void GUIDomainSampleVisitor::visit(const MultiLayer* node)
 {
     auto* multilayer_item = m_sampleModel->addMultiLayer();
-    multilayer_item->setItemName(node->sampleName().c_str());
+    multilayer_item->setSampleName(node->sampleName().c_str());
     multilayer_item->crossCorrLength().set(node->crossCorrLength());
     multilayer_item->setExternalField(node->externalField());
     m_levelToParentItem[depth()] = multilayer_item;
diff --git a/GUI/Model/From/SampleListModel.cpp b/GUI/Model/From/SampleListModel.cpp
index 09efed232d9..ca85759181b 100644
--- a/GUI/Model/From/SampleListModel.cpp
+++ b/GUI/Model/From/SampleListModel.cpp
@@ -63,11 +63,11 @@ QVariant SampleListModel::data(const QModelIndex& index, int role) const
             }
             descr.replace("\n", "<br>");
         }
-        return "<b>" + item->itemName() + "</b>" + descr;
+        return "<b>" + item->sampleName() + "</b>" + descr;
     }
 
     if (role == Qt::EditRole)
-        return item->itemName();
+        return item->sampleName();
 
     return QVariant();
 }
@@ -86,7 +86,7 @@ bool SampleListModel::setData(const QModelIndex& index, const QVariant& value, i
         return false;
 
     if (role == Qt::EditRole && index.column() == 0) {
-        itemForIndex(index)->setItemName(value.toString());
+        itemForIndex(index)->setSampleName(value.toString());
         emit dataChanged(index, index);
         return true;
     }
@@ -134,7 +134,7 @@ QModelIndex SampleListModel::createSample()
     const int row = m_sampleModel->multiLayerItems().size();
     beginInsertRows(QModelIndex(), row, row);
     auto* multilayer_item = m_sampleModel->addMultiLayer();
-    multilayer_item->setItemName(suggestName(topItemNames(m_sampleModel), "Sample"));
+    multilayer_item->setSampleName(suggestName(topItemNames(m_sampleModel), "Sample"));
     endInsertRows();
     return indexForItem(multilayer_item);
 }
@@ -149,7 +149,7 @@ QModelIndex SampleListModel::createSampleFromExamples(const QString& className,
 
     auto* sample = dynamic_cast<MultiLayerItem*>(
         GUI::ExamplesFactory::createSampleItems(className, m_sampleModel, materialModel));
-    sample->setItemName(title);
+    sample->setSampleName(title);
     sample->setDescription(description);
 
     endInsertRows();
diff --git a/GUI/Model/Job/JobModel.cpp b/GUI/Model/Job/JobModel.cpp
index 8130489f427..9b82fab22b0 100644
--- a/GUI/Model/Job/JobModel.cpp
+++ b/GUI/Model/Job/JobModel.cpp
@@ -87,7 +87,7 @@ JobItem* JobModel::addJob(const MultiLayerItem* multiLayerItem,
         GUI::Model::JobFunctions::muteMagnetizationData(jobItem);
     jobItem->copySimulationOptionsIntoJob(optionItem);
 
-    jobItem->setSampleName(multiLayerItem->itemName());
+    jobItem->setSampleName(multiLayerItem->sampleName());
 
     GUI::Model::ParameterTreeUtils::createParameterTree(jobItem, true);
 
diff --git a/GUI/Model/Job/JobModelFunctions.cpp b/GUI/Model/Job/JobModelFunctions.cpp
index b57df4b4f5b..f2477f5367f 100644
--- a/GUI/Model/Job/JobModelFunctions.cpp
+++ b/GUI/Model/Job/JobModelFunctions.cpp
@@ -126,7 +126,7 @@ void GUI::Model::JobFunctions::setupJobItemSampleData(JobItem* jobItem,
                                                       const MultiLayerItem* sampleItem)
 {
     MultiLayerItem* multilayer = jobItem->copySampleIntoJob(sampleItem);
-    multilayer->setItemName(MultiLayerItem::M_TYPE);
+    multilayer->setSampleName(MultiLayerItem::M_TYPE);
 
     // copy used materials into material container
     for (auto* itemWithMaterial : sampleItem->itemsWithMaterial()) {
diff --git a/GUI/Model/Sample/MultiLayerItem.cpp b/GUI/Model/Sample/MultiLayerItem.cpp
index 3962c28e6c8..5b783ba1e81 100644
--- a/GUI/Model/Sample/MultiLayerItem.cpp
+++ b/GUI/Model/Sample/MultiLayerItem.cpp
@@ -48,6 +48,16 @@ QVector<ItemWithMaterial*> MultiLayerItem::itemsWithMaterial() const
     return result;
 }
 
+QString MultiLayerItem::sampleName() const
+{
+    return itemName();
+}
+
+void MultiLayerItem::setSampleName(const QString& name)
+{
+    setItemName(name);
+}
+
 QString MultiLayerItem::description() const
 {
     return getItemValue(P_DESCRIPTION).toString();
diff --git a/GUI/Model/Sample/MultiLayerItem.h b/GUI/Model/Sample/MultiLayerItem.h
index 8467e30e550..b5e42623853 100644
--- a/GUI/Model/Sample/MultiLayerItem.h
+++ b/GUI/Model/Sample/MultiLayerItem.h
@@ -37,6 +37,9 @@ public:
 
     QVector<ItemWithMaterial*> itemsWithMaterial() const;
 
+    QString sampleName() const;
+    void setSampleName(const QString& name);
+
     QString description() const;
     void setDescription(const QString& description);
 
@@ -52,9 +55,10 @@ public:
     SessionItem* layersItem() const;
 
     //! Creates and inserts a layer at given index.
+    //!
     //! No properties etc. have been initialized, this  has to be done by the caller
-    //! If index = -1, create a layer at the end ot the list.
-    LayerItem* addLayer(int index);
+    //! If index = -1, create a layer at the end of the list.
+    LayerItem* addLayer(int index = -1);
 
     void removeLayer(LayerItem* item);
     void moveLayer(LayerItem* item, LayerItem* beforeThisLayer);
diff --git a/GUI/Model/To/DomainObjectBuilder.cpp b/GUI/Model/To/DomainObjectBuilder.cpp
index bf99d568664..0cb7689408f 100644
--- a/GUI/Model/To/DomainObjectBuilder.cpp
+++ b/GUI/Model/To/DomainObjectBuilder.cpp
@@ -22,12 +22,12 @@
 #include "GUI/Util/Error.h"
 
 std::unique_ptr<MultiLayer>
-GUI::Model::DomainObjectBuilder::buildMultiLayer(const MultiLayerItem& multilayer_item)
+GUI::Model::DomainObjectBuilder::buildMultiLayer(const MultiLayerItem& multiLayerItem)
 {
-    auto P_multilayer = GUI::Transform::ToDomain::createMultiLayer(multilayer_item);
-    for (LayerItem* child : multilayer_item.childrenOfType<LayerItem>()) {
-        auto P_layer = buildLayer(*child);
-        const auto roughness = child->roughness().currentItem();
+    auto P_multilayer = GUI::Transform::ToDomain::createMultiLayer(multiLayerItem);
+    for (auto* layerItem : multiLayerItem.layers()) {
+        auto P_layer = buildLayer(*layerItem);
+        const auto roughness = layerItem->roughness().currentItem();
         auto P_roughness = GUI::Transform::ToDomain::createLayerRoughness(roughness);
         if (P_layer) {
             if (P_roughness)
diff --git a/GUI/View/SampleDesigner/SampleListView.cpp b/GUI/View/SampleDesigner/SampleListView.cpp
index 7021d7b8d8b..698fc3430d9 100644
--- a/GUI/View/SampleDesigner/SampleListView.cpp
+++ b/GUI/View/SampleDesigner/SampleListView.cpp
@@ -171,7 +171,7 @@ void SampleListView::editNameAndDescription(MultiLayerItem* item)
 
     auto* layout = new QFormLayout(&dlg);
     auto* nameEdit = new QLineEdit(&dlg);
-    nameEdit->setText(item->itemName());
+    nameEdit->setText(item->sampleName());
     nameEdit->selectAll();
 
     auto* descriptionEdit = new QTextEdit(&dlg);
diff --git a/GUI/View/Toplevel/SimulationView.cpp b/GUI/View/Toplevel/SimulationView.cpp
index 1e5a2f3c1d0..d9e39265eed 100644
--- a/GUI/View/Toplevel/SimulationView.cpp
+++ b/GUI/View/Toplevel/SimulationView.cpp
@@ -34,6 +34,16 @@
 #include <QMessageBox>
 #include <thread>
 
+namespace {
+QStringList sampleNames(const QVector<MultiLayerItem*> multilayerItems)
+{
+    QStringList result;
+    for (const auto* p : multilayerItems)
+        result.append(p->sampleName());
+    return result;
+}
+} // namespace
+
 SimulationView::SimulationView(QWidget* parent, ProjectDocument* document)
     : QWidget(parent), m_ui(new Ui::SimulationView), m_document(document)
 {
@@ -111,7 +121,7 @@ void SimulationView::writeOptionsToUI()
     // -- selection group
     using GUI::Model::ItemUtils::itemNames;
     updateSelection(m_ui->instrumentCombo, itemNames(instrumentItems()));
-    updateSelection(m_ui->sampleCombo, itemNames(multiLayerItems()));
+    updateSelection(m_ui->sampleCombo, sampleNames(multiLayerItems()));
     updateSelection(m_ui->realDataCombo, itemNames(realDataItems()), true);
 
     // -- options group
diff --git a/Tests/Unit/GUI/TestMapperForItem.cpp b/Tests/Unit/GUI/TestMapperForItem.cpp
index 808cc14ee96..ece294796a9 100644
--- a/Tests/Unit/GUI/TestMapperForItem.cpp
+++ b/Tests/Unit/GUI/TestMapperForItem.cpp
@@ -125,7 +125,7 @@ TEST_F(TestMapperForItem, onPropertyChange)
     Widget w;
     SampleModel model;
     auto* multilayer = model.insertItem<MultiLayerItem>();
-    auto* layer = model.insertItem<LayerItem>(multilayer);
+    auto* layer = multilayer->addLayer();
 
     // Mapper is looking on child; set property of child
     setItem(layer, &w);
diff --git a/Tests/Unit/GUI/TestModelUtils.cpp b/Tests/Unit/GUI/TestModelUtils.cpp
index 89fdce2e027..a1bd133422f 100644
--- a/Tests/Unit/GUI/TestModelUtils.cpp
+++ b/Tests/Unit/GUI/TestModelUtils.cpp
@@ -21,10 +21,10 @@ TEST_F(TestModelUtils, topItemNames)
 
     // inserting three top items
     auto* item = model.insertItem<MultiLayerItem>();
-    item->setItemName("name1");
+    item->setSampleName("name1");
     model.insertItem<LayerItem>();
     item = model.insertItem<MultiLayerItem>();
-    item->setItemName("name2");
+    item->setSampleName("name2");
 
     // checking names of items of certain type
     ASSERT_EQ(GUI::Model::ItemUtils::topItemNames(&model, MultiLayerItem::M_TYPE).size(), 2);
diff --git a/Tests/Unit/GUI/TestMultiLayerItem.cpp b/Tests/Unit/GUI/TestMultiLayerItem.cpp
index 209171ce56c..bf2f2bd60cb 100644
--- a/Tests/Unit/GUI/TestMultiLayerItem.cpp
+++ b/Tests/Unit/GUI/TestMultiLayerItem.cpp
@@ -17,8 +17,8 @@ TEST_F(TestMultiLayerItem, twoLayerSystem)
     SampleModel model;
 
     auto* multilayer = model.addMultiLayer();
-    auto* top = model.insertItem<LayerItem>(multilayer);
-    auto* bottom = model.insertItem<LayerItem>(multilayer);
+    auto* top = multilayer->addLayer();
+    auto* bottom = multilayer->addLayer();
 
     // Thickness property should be disabled for top and bottom layers
     EXPECT_FALSE(top->thicknessItem()->isEnabled());
@@ -46,9 +46,9 @@ TEST_F(TestMultiLayerItem, threeLayerSystem)
     SampleModel model;
 
     auto* multilayer = model.addMultiLayer();
-    auto* top = model.insertItem<LayerItem>(multilayer);
-    auto* middle = model.insertItem<LayerItem>(multilayer);
-    auto* bottom = model.insertItem<LayerItem>(multilayer);
+    auto* top = multilayer->addLayer();
+    auto* middle = multilayer->addLayer();
+    auto* bottom = multilayer->addLayer();
 
     // Thickness property should be disabled for top and bottom layers and enabled for middle
     EXPECT_FALSE(top->thicknessItem()->isEnabled());
@@ -81,9 +81,9 @@ TEST_F(TestMultiLayerItem, movingMiddleLayerOnTop)
     SampleModel model;
 
     auto* multilayer = model.addMultiLayer();
-    auto* top = model.insertItem<LayerItem>(multilayer);
-    auto* middle = model.insertItem<LayerItem>(multilayer);
-    auto* bottom = model.insertItem<LayerItem>(multilayer);
+    auto* top = multilayer->addLayer();
+    auto* middle = multilayer->addLayer();
+    auto* bottom = multilayer->addLayer();
 
     const double thickness = 10.0;
     middle->thickness().set(thickness);
@@ -104,10 +104,10 @@ TEST_F(TestMultiLayerItem, movingMiddleLayerOnTop)
     EXPECT_EQ(bottom->thickness(), 0.0);
 
     // Moving middle layer to top
-    model.moveItem(middle, multilayer, 0, multilayer->tagFromItem(multilayer->layersItem()));
+    multilayer->moveLayer(middle, top);
     // checking that middle is top now, and top layer is middle now
-    EXPECT_EQ(middle, multilayer->childrenOfType<LayerItem>().at(0));
-    EXPECT_EQ(top, multilayer->childrenOfType<LayerItem>().at(1));
+    EXPECT_EQ(middle, multilayer->layers().at(0));
+    EXPECT_EQ(top, multilayer->layers().at(1));
 
     // Thickness and roughness of middle layer should be disabled now
     EXPECT_FALSE(middle->thicknessItem()->isEnabled());
diff --git a/Tests/Unit/GUI/TestParaCrystalItems.cpp b/Tests/Unit/GUI/TestParaCrystalItems.cpp
index 97f2de5b54e..3381ed241e5 100644
--- a/Tests/Unit/GUI/TestParaCrystalItems.cpp
+++ b/Tests/Unit/GUI/TestParaCrystalItems.cpp
@@ -74,7 +74,7 @@ TEST_F(TestParaCrystalItems, Inference2DRotationAngleToggle)
 {
     SampleModel model;
     auto* multilayer = model.insertItem<MultiLayerItem>();
-    auto* layer = model.insertItem<LayerItem>(multilayer);
+    auto* layer = multilayer->addLayer();
     auto* layout = model.insertItem<ParticleLayoutItem>(layer);
 
     auto* interference = layout->createInterference<Interference2DParaCrystalItem>();
diff --git a/Tests/Unit/GUI/TestSessionModel.cpp b/Tests/Unit/GUI/TestSessionModel.cpp
index e5671222876..537e1a1851a 100644
--- a/Tests/Unit/GUI/TestSessionModel.cpp
+++ b/Tests/Unit/GUI/TestSessionModel.cpp
@@ -51,10 +51,10 @@ TEST_F(TestSessionModel, SampleModelCopy)
 
     SampleModel model1;
     auto* multilayer = model1.insertItem<MultiLayerItem>();
-    multilayer->setItemName("multilayer");
-    model1.insertItem<LayerItem>(multilayer);
+    multilayer->setSampleName("multilayer");
+    multilayer->addLayer();
     auto* multilayer2 = model1.insertItem<MultiLayerItem>();
-    multilayer2->setItemName("multilayer2");
+    multilayer2->setSampleName("multilayer2");
 
     QString buffer1;
     QXmlStreamWriter writer1(&buffer1);
@@ -74,16 +74,16 @@ TEST_F(TestSessionModel, SampleModelPartialCopy)
 
     SampleModel model1;
     auto* multilayer1 = model1.insertItem<MultiLayerItem>();
-    multilayer1->setItemName("multilayer1");
+    multilayer1->setSampleName("multilayer1");
     model1.insertItem<LayerItem>(multilayer1);
 
     auto* multilayer2 = model1.insertItem<MultiLayerItem>();
-    multilayer2->setItemName("multilayer2");
+    multilayer2->setSampleName("multilayer2");
 
     std::unique_ptr<SampleModel> model2(model1.createCopy(multilayer1));
     SessionItem* result = model2->itemForIndex(model2->index(0, 0, QModelIndex()));
 
-    EXPECT_EQ(result->itemName(), multilayer1->itemName());
+    EXPECT_EQ(result->itemName(), multilayer1->sampleName());
     EXPECT_EQ(result->modelType(), multilayer1->modelType());
 }
 
@@ -131,7 +131,7 @@ TEST_F(TestSessionModel, copyItem)
 
     SampleModel sampleModel;
     auto* multilayer1 = sampleModel.insertItem<MultiLayerItem>();
-    multilayer1->setItemName("multilayer1");
+    multilayer1->setSampleName("multilayer1");
     sampleModel.insertItem<LayerItem>(multilayer1);
 
     InstrumentModel instrumentModel;
-- 
GitLab