Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the abstract class CreateConnectedElementI #1047

Open
wants to merge 2 commits into
base: cpacs_creator_dev_merge
Choose a base branch
from
Open

Refactor the abstract class CreateConnectedElementI #1047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b8166b
Refactor the abstract class CreateConnectedElementI by adding std::va…
svengoldberg Dec 19, 2024
e9d6cdd
Refactor the std::variant way by using an template constructor choosi…
svengoldberg Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions TIGLViewer/src/ModificatorContainerWidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,10 @@ void ModificatorContainerWidget::setSectionModificator(QList<tigl::CTiglSectionE
}


void ModificatorContainerWidget::setSectionsModificator(tigl::CreateConnectedElementI& element)
void ModificatorContainerWidget::setSectionsModificator(Ui::ElementModificatorInterface& element)
{
hideAllSpecializedWidgets();
ui->sectionsModificator->setCreateConnectedElementI(element);
ui->sectionsModificator->setCreateConnectedElement(element);
ui->sectionsModificator->setVisible(true);
currentModificator = ui->sectionModificator;
}
Expand Down
3 changes: 2 additions & 1 deletion TIGLViewer/src/ModificatorContainerWidget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "modificators/ModificatorWingWidget.h"
#include "modificators/ModificatorFuselageWidget.h"
#include "modificators/ModificatorTransformationWidget.h"
#include "modificators/ModificatorSectionsWidget.h"
#include "CCPACSFuselages.h"
#include "CPACSWing.h"
#include "CCPACSWings.h"
Expand Down Expand Up @@ -78,7 +79,7 @@ public slots:
void setFuselagesModificator(tigl::CCPACSFuselages& fuselages);
void setElementModificator(tigl::CTiglSectionElement& element);
void setSectionModificator(QList<tigl::CTiglSectionElement*> elements);
void setSectionsModificator(tigl::CreateConnectedElementI& conntedElementI);
void setSectionsModificator(Ui::ElementModificatorInterface& conntedElementI);
// for positioning, we need two different calls for wing and for fuselage. Otherwise, we miss to invalidate the
// associated wing or fuselage
void setPositioningModificator(tigl::CCPACSWing& wing, tigl::CCPACSPositioning& positioning);
Expand Down
30 changes: 16 additions & 14 deletions TIGLViewer/src/ModificatorManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include "CTiglSectionElement.h"
#include "CCPACSFuselageSection.h"
#include "CCPACSWingSection.h"
#include "CreateConnectedElementI.h"
#include "CCPACSPositioning.h"
#include "CTiglStandardizer.h"
#include "TIGLViewerContext.h"
Expand Down Expand Up @@ -150,21 +149,24 @@ void ModificatorManager::dispatch(cpcr::CPACSTreeItem* item)
std::string bodyUID = item->getParent()->getUid(); // return the fuselage or wing uid
tigl::CTiglUIDManager& uidManager = doc->GetConfiguration().GetUIDManager();
tigl::CTiglUIDManager::TypedPtr typePtr = uidManager.ResolveObject(bodyUID);
tigl::CreateConnectedElementI * elementI = nullptr;

if (typePtr.type == &typeid(tigl::CCPACSWing)) {
tigl::CCPACSWing &wing = *reinterpret_cast<tigl::CCPACSWing *>(typePtr.ptr);
elementI = dynamic_cast<tigl::CreateConnectedElementI* >(&wing);
}
else if (typePtr.type == &typeid(tigl::CCPACSFuselage)) {
tigl::CCPACSFuselage &fuselage = *reinterpret_cast<tigl::CCPACSFuselage *>(typePtr.ptr);
elementI = dynamic_cast<tigl::CreateConnectedElementI* >(&fuselage);
}
else {
LOG(ERROR) << "ModificatorManager:: Unexpected sections type!";
}
auto get_element_interface = [](tigl::CTiglUIDManager::TypedPtr const& ptr) {
if (ptr.type == &typeid(tigl::CCPACSWing)) {
tigl::CCPACSWing &wing = *reinterpret_cast<tigl::CCPACSWing*>(ptr.ptr);
return Ui::ElementModificatorInterface(wing);
}
else if (ptr.type == &typeid(tigl::CCPACSFuselage)) {
tigl::CCPACSFuselage &fuselage = *reinterpret_cast<tigl::CCPACSFuselage*>(ptr.ptr);
return Ui::ElementModificatorInterface(fuselage);
}
else {
LOG(ERROR) << "ModificatorManager:: Unexpected sections type!";
return Ui::ElementModificatorInterface(tigl::CCPACSFuselage(nullptr, nullptr));
}
};

modificatorContainerWidget->setSectionsModificator(*elementI);
auto element = get_element_interface(typePtr);
modificatorContainerWidget->setSectionsModificator(element);
}
else if (item->getType() == "positioning" ) {
tigl::CTiglUIDManager& uidManager = doc->GetConfiguration().GetUIDManager();
Expand Down
22 changes: 12 additions & 10 deletions TIGLViewer/src/modificators/ModificatorSectionsWidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ ModificatorSectionsWidget::ModificatorSectionsWidget(QWidget* parent)
, ui(new Ui::ModificatorSectionsWidget)
{
ui->setupUi(this);
createConnectedElementI = nullptr;
createConnectedElement = nullptr;
connect(ui->addConnectedElementBtn, SIGNAL(pressed()), this, SLOT(execNewConnectedElementDialog()));
connect(ui->deleteConnectedElementBtn, SIGNAL(pressed()), this, SLOT(execDeleteConnectedElementDialog()));
}
Expand All @@ -40,19 +40,20 @@ ModificatorSectionsWidget::~ModificatorSectionsWidget()
delete ui;
}

void ModificatorSectionsWidget::setCreateConnectedElementI(tigl::CreateConnectedElementI& elementI)
void ModificatorSectionsWidget::setCreateConnectedElement(Ui::ElementModificatorInterface& element)
{
createConnectedElementI = &elementI;
createConnectedElement = &element;
}

void ModificatorSectionsWidget::execNewConnectedElementDialog()
{
if (createConnectedElementI == nullptr) {
if (createConnectedElement == nullptr) {
LOG(ERROR) << "ModificatorSectionsWidget:: is not correctly set!";
return;
}

std::vector<std::string> elementUIDs = createConnectedElementI->GetOrderedConnectedElement();
std::vector<std::string> elementUIDs = createConnectedElement->GetOrderedConnectedElement();

QStringList elementUIDsQList;
for (int i = 0; i < elementUIDs.size(); i++) {
elementUIDsQList.push_back(elementUIDs.at(i).c_str());
Expand All @@ -64,10 +65,10 @@ void ModificatorSectionsWidget::execNewConnectedElementDialog()
NewConnectedElementDialog::Where where = newElementDialog.getWhere();
try {
if (where == NewConnectedElementDialog::Before) {
createConnectedElementI->CreateNewConnectedElementBefore(startUID);
createConnectedElement->CreateNewConnectedElementBefore(startUID);
}
else if (where == NewConnectedElementDialog::After) {
createConnectedElementI->CreateNewConnectedElementAfter(startUID);
createConnectedElement->CreateNewConnectedElementAfter(startUID);
}
}
catch (const tigl::CTiglError& err) {
Expand All @@ -87,12 +88,13 @@ void ModificatorSectionsWidget::execNewConnectedElementDialog()
void ModificatorSectionsWidget::execDeleteConnectedElementDialog()
{

if (createConnectedElementI == nullptr) {
if (createConnectedElement == nullptr) {
LOG(ERROR) << "ModificatorWingsWidget::execDeleteWingDialog: wings is not set!";
return;
}

std::vector<std::string> elementUIDs = createConnectedElementI->GetOrderedConnectedElement();
std::vector<std::string> elementUIDs = createConnectedElement->GetOrderedConnectedElement();

QStringList elementUIDsQList;
for (int i = 0; i < elementUIDs.size(); i++) {
elementUIDsQList.push_back(elementUIDs.at(i).c_str());
Expand All @@ -102,7 +104,7 @@ void ModificatorSectionsWidget::execDeleteConnectedElementDialog()
if (deleteDialog.exec() == QDialog::Accepted) {
QString uid = deleteDialog.getUIDToDelete();
try {
createConnectedElementI->DeleteConnectedElement(uid.toStdString());
createConnectedElement->DeleteConnectedElement(uid.toStdString());
}
catch (const tigl::CTiglError& err) {
TIGLViewerErrorDialog errDialog(this);
Expand Down
41 changes: 38 additions & 3 deletions TIGLViewer/src/modificators/ModificatorSectionsWidget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,45 @@
#define MODIFICATORSECTIONSWIDGET_H

#include <QWidget>
#include "CreateConnectedElementI.h"
#include <string>
#include "CCPACSFuselage.h"
#include "CCPACSWing.h"

namespace Ui
{

// Interface-like structure to account for different possible object types whose member variables may be adapted by the CPACSCreator.
// It is currently used for CCPACSFuselage and CCPACSWing.
// Could be extended by Duct, Pylon, Tank, etc. in the future (observe: The respective classes need to define the listed functions).
struct ElementModificatorInterface
{
// Here, functions are defined as member variables calling the 'right' (depending on present data type) function from CCPACSFuselage, CCPACSWing, etc. via lambdas
template <typename T>
ElementModificatorInterface(T&& t)
: CreateNewConnectedElementAfter(
[&t](std::string str){ return t.CreateNewConnectedElementAfter(str); }
)
, CreateNewConnectedElementBefore(
[&t](std::string str){ return t.CreateNewConnectedElementBefore(str); }
)
, CreateNewConnectedElementBetween(
[&t](std::string str1, std::string str2){ return t.CreateNewConnectedElementBetween(str1, str2); }
)
, DeleteConnectedElement(
[&t](std::string str){ return t.DeleteConnectedElement(str); }
)
, GetOrderedConnectedElement(
[&t](){ return t.GetOrderedConnectedElement(); }
)
{}

std::function<void(std::string)> CreateNewConnectedElementAfter;
std::function<void(std::string)> CreateNewConnectedElementBefore;
std::function<void(std::string, std::string)> CreateNewConnectedElementBetween;
std::function<void(std::string)> DeleteConnectedElement;
std::function<std::vector<std::string>()> GetOrderedConnectedElement;
};

class ModificatorSectionsWidget;
}

Expand All @@ -42,11 +77,11 @@ public slots:
explicit ModificatorSectionsWidget(QWidget* parent = nullptr);
~ModificatorSectionsWidget();

void setCreateConnectedElementI(tigl::CreateConnectedElementI& elementI);
void setCreateConnectedElement(Ui::ElementModificatorInterface& element);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't the argument be const?

Suggested change
void setCreateConnectedElement(Ui::ElementModificatorInterface& element);
void setCreateConnectedElement(Ui::ElementModificatorInterface const& element);


private:
Ui::ModificatorSectionsWidget* ui;
tigl::CreateConnectedElementI* createConnectedElementI;
Ui::ElementModificatorInterface* createConnectedElement;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe with the current changes, we should store by value here. The ElementModificatorInterface goes out of scope at the end of the function where it is created and the pointer can be dangling. In addition, the ElementModificatorInterface already stores the fuselage or wing by reference.

};

#endif // MODIFICATORSECTIONSWIDGET_H
13 changes: 6 additions & 7 deletions src/fuselage/CCPACSFuselage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#include "CCPACSGuideCurve.h"
#include "CTiglFuselageConnection.h"
#include "Cache.h"
#include "CreateConnectedElementI.h"

#include "TopoDS_Shape.hxx"
#include "TopoDS_Compound.hxx"
Expand All @@ -48,7 +47,7 @@ namespace tigl
class CCPACSConfiguration;
class CCPACSFuselageStringerFramePosition;

class CCPACSFuselage : public generated::CPACSFuselage, public CTiglRelativelyPositionedComponent, public CreateConnectedElementI
class CCPACSFuselage : public generated::CPACSFuselage, public CTiglRelativelyPositionedComponent
{
public:
// Constructor
Expand Down Expand Up @@ -171,7 +170,7 @@ class CCPACSFuselage : public generated::CPACSFuselage, public CTiglRelativelyPo
*
* @param startElementUID
*/
TIGL_EXPORT void CreateNewConnectedElementAfter(std::string startElementUID) override;
TIGL_EXPORT void CreateNewConnectedElementAfter(std::string startElementUID);

/**
* Create a new section, a new element and connect the element to the "startElement".
Expand All @@ -181,20 +180,20 @@ class CCPACSFuselage : public generated::CPACSFuselage, public CTiglRelativelyPo
*
* @param startElementUID
*/
TIGL_EXPORT void CreateNewConnectedElementBefore(std::string startElementUID) override;
TIGL_EXPORT void CreateNewConnectedElementBefore(std::string startElementUID);

/**
*Create a new section, a new element and place the new element between the startElement and the endElement.
* @remark The startElement and endElement must be connected by a segment.
* @param startElementUID
* @param endElementUID
*/
TIGL_EXPORT void CreateNewConnectedElementBetween(std::string startElementUID, std::string endElementUID) override;
TIGL_EXPORT void CreateNewConnectedElementBetween(std::string startElementUID, std::string endElementUID);


TIGL_EXPORT void DeleteConnectedElement(std::string elementUID) override;
TIGL_EXPORT void DeleteConnectedElement(std::string elementUID);

TIGL_EXPORT std::vector<std::string> GetOrderedConnectedElement() override;
TIGL_EXPORT std::vector<std::string> GetOrderedConnectedElement();

TIGL_EXPORT std::vector<tigl::CTiglSectionElement*> GetCTiglElements() ;

Expand Down
84 changes: 0 additions & 84 deletions src/geometry/CreateConnectedElementI.h
View file
Open in desktop

This file was deleted.

Loading
Loading