Skip to content

[Core] Removing PointsArrayVector explicit cast in geometry#14248

Open
roigcarlo wants to merge 5 commits intomasterfrom
core/remove-geometry-cast
Open

[Core] Removing PointsArrayVector explicit cast in geometry#14248
roigcarlo wants to merge 5 commits intomasterfrom
core/remove-geometry-cast

Conversation

@roigcarlo
Copy link
Member

📝 Description
@KratosMultiphysics/technical-committee What we discussed. After going through the code, I believe that 45% of this changes are due to people creating geometries instead of NodeArrays because they did not know that was the argument; 45% of uses that do not care about being a reference or a point and the other 10% legit usages.

Still, i just did a 1:1 translation, but feel free to comment if you believe we could improve the overall code changing the geometry reference to pointer or an actual list of nodes.

@roigcarlo roigcarlo requested review from a team as code owners March 2, 2026 15:19

//generate new element by cloning the base one
Element::Pointer p_element = it->Create(current_id, geom, it->pGetProperties());
Element::Pointer p_element = it->Create(current_id, geom.Points(), it->pGetProperties());
Copy link
Member Author

Choose a reason for hiding this comment

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

For example, cases like this one rely on the casting purely because of the sake of it. The same code could be implemented using a list of nodes directly...

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I imagine ppl think that things are implemented for a reason and they can be used so I don't blame them. I really hate how many ambiguous, unnecessary things we define.

@roigcarlo roigcarlo requested a review from a team as a code owner March 3, 2026 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants