From ee8b29b365600c768d621574a728198d6b40cd56 Mon Sep 17 00:00:00 2001
From: Matthias Puchner <github@mpuchner.de>
Date: Thu, 25 Nov 2021 07:19:47 +0100
Subject: [PATCH] reduce SessionItem exposure in MaterialItem
---
GUI/Model/Material/MaterialItem.cpp | 10 ++++++++++
GUI/Model/Material/MaterialItem.h | 3 +++
GUI/Model/Material/MaterialModel.cpp | 8 ++++----
GUI/Model/Sample/ItemWithMaterial.cpp | 6 ++----
GUI/View/MaterialEditor/MaterialEditorDialog.cpp | 2 +-
GUI/View/MaterialEditor/MaterialEditorModel.cpp | 4 ++--
Tests/Unit/GUI/TestMaterialModel.cpp | 12 ++++++------
7 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/GUI/Model/Material/MaterialItem.cpp b/GUI/Model/Material/MaterialItem.cpp
index fa2d9e14791..a122144b50f 100644
--- a/GUI/Model/Material/MaterialItem.cpp
+++ b/GUI/Model/Material/MaterialItem.cpp
@@ -125,6 +125,16 @@ complex_t MaterialItem::scatteringLengthDensity() const
return complex_t(real, imag);
}
+QString MaterialItem::name() const
+{
+ return itemName();
+}
+
+void MaterialItem::setName(const QString& name)
+{
+ setItemName(name);
+}
+
QString MaterialItem::identifier() const
{
return getItemValue(P_IDENTIFIER).toString();
diff --git a/GUI/Model/Material/MaterialItem.h b/GUI/Model/Material/MaterialItem.h
index 5a5eb40898b..c030539f440 100644
--- a/GUI/Model/Material/MaterialItem.h
+++ b/GUI/Model/Material/MaterialItem.h
@@ -55,6 +55,9 @@ public:
/// \pre ! hasRefractiveIndex
complex_t scatteringLengthDensity() const;
+ QString name() const;
+ void setName(const QString& name);
+
QString identifier() const;
void setIdentifier(const QString& id);
diff --git a/GUI/Model/Material/MaterialModel.cpp b/GUI/Model/Material/MaterialModel.cpp
index f77f24d16b0..6d04eaa6be2 100644
--- a/GUI/Model/Material/MaterialModel.cpp
+++ b/GUI/Model/Material/MaterialModel.cpp
@@ -60,7 +60,7 @@ MaterialModel* MaterialModel::createCopy(SessionItem* parent)
MaterialItem* MaterialModel::addRefractiveMaterial(const QString& name, double delta, double beta)
{
auto* materialItem = insertItem<MaterialItem>();
- materialItem->setItemName(name);
+ materialItem->setName(name);
materialItem->setColor(suggestMaterialColor(name));
materialItem->setRefractiveIndex(delta, beta);
return materialItem;
@@ -69,7 +69,7 @@ MaterialItem* MaterialModel::addRefractiveMaterial(const QString& name, double d
MaterialItem* MaterialModel::addSLDMaterial(const QString& name, double sld, double abs_term)
{
auto* materialItem = insertItem<MaterialItem>();
- materialItem->setItemName(name);
+ materialItem->setName(name);
materialItem->setColor(suggestMaterialColor(name));
materialItem->setScatteringLengthDensity(complex_t(sld, abs_term));
return materialItem;
@@ -78,7 +78,7 @@ MaterialItem* MaterialModel::addSLDMaterial(const QString& name, double sld, dou
MaterialItem* MaterialModel::materialFromName(const QString& name)
{
for (auto* materialItem : topItems<MaterialItem>())
- if (materialItem->itemName() == name)
+ if (materialItem->name() == name)
return materialItem;
return nullptr;
@@ -100,7 +100,7 @@ MaterialItem* MaterialModel::cloneMaterial(MaterialItem* material)
auto* clonedMaterial = copyItem(material, nullptr);
clonedMaterial->setIdentifier(QUuid::createUuid().toString());
- clonedMaterial->setItemName(material->itemName() + " (copy)");
+ clonedMaterial->setName(material->name() + " (copy)");
return clonedMaterial;
}
diff --git a/GUI/Model/Sample/ItemWithMaterial.cpp b/GUI/Model/Sample/ItemWithMaterial.cpp
index 45522d5b998..dfbc3fb6d2e 100644
--- a/GUI/Model/Sample/ItemWithMaterial.cpp
+++ b/GUI/Model/Sample/ItemWithMaterial.cpp
@@ -53,11 +53,9 @@ QString ItemWithMaterial::materialName() const
{
const auto* const parentJob = GUI::Model::JobFunctions::findJobItem(this);
if (parentJob)
- return parentJob->materialContainerItem()
- ->findMaterialById(materialIdentifier())
- ->itemName();
+ return parentJob->materialContainerItem()->findMaterialById(materialIdentifier())->name();
- return GUI::MaterialUtil::findMaterial(materialIdentifier())->itemName();
+ return GUI::MaterialUtil::findMaterial(materialIdentifier())->name();
}
QString ItemWithMaterial::materialIdentifier() const
diff --git a/GUI/View/MaterialEditor/MaterialEditorDialog.cpp b/GUI/View/MaterialEditor/MaterialEditorDialog.cpp
index 974b07f3a64..d38846b5e7d 100644
--- a/GUI/View/MaterialEditor/MaterialEditorDialog.cpp
+++ b/GUI/View/MaterialEditor/MaterialEditorDialog.cpp
@@ -242,7 +242,7 @@ void MaterialEditorDialog::fill()
m_ui->refractiveGroupBox->setVisible(materialItem->hasRefractiveIndex());
m_ui->sldGroupBox->setVisible(!materialItem->hasRefractiveIndex());
- m_ui->nameEdit->setText(materialItem->itemName());
+ m_ui->nameEdit->setText(materialItem->name());
m_ui->colorInfo->setText(QString("[%1, %2, %3] (%4)")
.arg(materialItem->color().red())
.arg(materialItem->color().green())
diff --git a/GUI/View/MaterialEditor/MaterialEditorModel.cpp b/GUI/View/MaterialEditor/MaterialEditorModel.cpp
index 53f551b56b3..8b14b2272c5 100644
--- a/GUI/View/MaterialEditor/MaterialEditorModel.cpp
+++ b/GUI/View/MaterialEditor/MaterialEditorModel.cpp
@@ -62,7 +62,7 @@ QVariant MaterialEditorModel::data(const QModelIndex& index, int role /*= Qt::Di
if (role == Qt::DisplayRole) {
switch (index.column()) {
case NAME:
- return material->itemName();
+ return material->name();
case TYPE:
return material->hasRefractiveIndex() ? "Refractive based" : "SLD based";
@@ -99,7 +99,7 @@ QVariant MaterialEditorModel::data(const QModelIndex& index, int role /*= Qt::Di
void MaterialEditorModel::setMaterialItemName(const QModelIndex& index, const QString& name)
{
- materialFromIndex(index)->setItemName(name);
+ materialFromIndex(index)->setName(name);
emit dataChanged(index, index);
}
diff --git a/Tests/Unit/GUI/TestMaterialModel.cpp b/Tests/Unit/GUI/TestMaterialModel.cpp
index 1310b74d7d2..5b67e7ae04c 100644
--- a/Tests/Unit/GUI/TestMaterialModel.cpp
+++ b/Tests/Unit/GUI/TestMaterialModel.cpp
@@ -21,7 +21,7 @@ TEST_F(TestMaterialModel, addRefractiveMaterial)
EXPECT_EQ(model->materialItems().size(), 1);
EXPECT_EQ(model->itemForIndex(material->index()), material);
- EXPECT_EQ(material->itemName(), name);
+ EXPECT_EQ(material->name(), name);
EXPECT_TRUE(material->hasRefractiveIndex());
EXPECT_EQ(material->refractiveIndexDelta(), delta);
EXPECT_EQ(material->refractiveIndexBeta(), beta);
@@ -40,7 +40,7 @@ TEST_F(TestMaterialModel, addSLDMaterial)
EXPECT_EQ(model->materialItems().size(), 1);
EXPECT_EQ(model->itemForIndex(material->index()), material);
- EXPECT_EQ(material->itemName(), name);
+ EXPECT_EQ(material->name(), name);
EXPECT_FALSE(material->hasRefractiveIndex());
EXPECT_EQ(material->scatteringLengthDensity(), complex_t(sld_real, sld_imag));
}
@@ -66,7 +66,7 @@ TEST_F(TestMaterialModel, cloneMaterialRefractive)
EXPECT_NE(clonedMaterial->identifier(), material->identifier());
// checking name of cloned material
- EXPECT_EQ(material->itemName() + " (copy)", clonedMaterial->itemName());
+ EXPECT_EQ(material->name() + " (copy)", clonedMaterial->name());
EXPECT_EQ(material->hasRefractiveIndex(), clonedMaterial->hasRefractiveIndex());
EXPECT_EQ(material->refractiveIndexDelta(), delta);
EXPECT_EQ(material->refractiveIndexBeta(), beta);
@@ -93,7 +93,7 @@ TEST_F(TestMaterialModel, cloneMaterialSLD)
EXPECT_NE(clonedMaterial->identifier(), material->identifier());
// checking name of cloned material
- EXPECT_EQ(material->itemName() + " (copy)", clonedMaterial->itemName());
+ EXPECT_EQ(material->name() + " (copy)", clonedMaterial->name());
EXPECT_EQ(material->hasRefractiveIndex(), clonedMaterial->hasRefractiveIndex());
EXPECT_EQ(material->scatteringLengthDensity(), complex_t(real, imag));
}
@@ -117,8 +117,8 @@ TEST_F(TestMaterialModel, materialFromName)
MaterialModel model;
MaterialItem* material1 = model.addRefractiveMaterial("aaa", 1.0, 2.0);
MaterialItem* material2 = model.addRefractiveMaterial("bbb", 3.0, 4.0);
- EXPECT_EQ(material1, model.materialFromName(material1->itemName()));
- EXPECT_EQ(material2, model.materialFromName(material2->itemName()));
+ EXPECT_EQ(material1, model.materialFromName(material1->name()));
+ EXPECT_EQ(material2, model.materialFromName(material2->name()));
EXPECT_EQ(nullptr, model.materialFromName("non-existing-name"));
}
--
GitLab