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.

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

[GeoMechanicsAnalysis] separated start of code cleaning U Pw diff order element #11820

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

WPK4FEM
Copy link
Contributor

@WPK4FEM WPK4FEM commented Nov 17, 2023

📝 Description
Start of reducing code smells in U Pw diff order element

Has been split of from the incremental linear material where this accidentally was included.
SonarCloud now shows 0 codesmells on this.

@WPK4FEM WPK4FEM added Refactor When code is moved or rewrote keeping the same behavior GeoMechanics Issues related to the GeoMechanicsApplication labels Nov 17, 2023
@WPK4FEM WPK4FEM requested a review from rfaasse November 17, 2023 12:19
@WPK4FEM WPK4FEM self-assigned this Nov 17, 2023
@WPK4FEM WPK4FEM enabled auto-merge (squash) November 17, 2023 12:42
Copy link
Contributor

@rfaasse rfaasse left a comment

Choose a reason for hiding this comment

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

Thanks for splitting this part out of the other PR, this makes it easier to see what's going on and thanks for incorporating the comments. I still see room for improvement within the files, but I think this is a very nice first step in cleaning this class up.

I have just one question left about changes to a part which might not be sufficiently tested. Other than that, this is ready to be merged

Comment on lines +584 to +597
for ( SizeType i = 0; i < NumUNodes; ++i ) {
rResult[Index] = rGeom[i].GetDof( DISPLACEMENT_X ).EquationId();
++Index;
rResult[Index] = rGeom[i].GetDof( DISPLACEMENT_Y ).EquationId();
++Index;
if (Dim > 2) {
rResult[Index] = rGeom[i].GetDof( DISPLACEMENT_Z ).EquationId();
++Index;
}
}
for ( SizeType i = 0; i < NumPNodes; ++i ){
rResult[Index] = rGeom[i].GetDof( WATER_PRESSURE ).EquationId();
++Index;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the improvement in this part, let's keep in mind for a next PR that we can extract a function here, since it seems identical for velocity and acceleration

Copy link
Contributor

@rfaasse rfaasse left a comment

Choose a reason for hiding this comment

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

Thanks for the clean-up, looks good to me!

@WPK4FEM WPK4FEM merged commit 6cad4e0 into master Nov 17, 2023
16 checks passed
@WPK4FEM WPK4FEM deleted the geo/codesmells_diff_order branch November 17, 2023 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GeoMechanics Issues related to the GeoMechanicsApplication Refactor When code is moved or rewrote keeping the same behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants