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

Make the protected object public to use its functions elsewhere. #5187

Merged
merged 2 commits into from Jul 8, 2023

Conversation

alarshi
Copy link
Contributor

@alarshi alarshi commented Jul 7, 2023

We use the object slab_boundary in our material model to access slab_depths and slab_thickness. In order to do this, this PR allows public access for the object.

Before your first pull request:

For all pull requests:

@bangerth
Copy link
Contributor

bangerth commented Jul 7, 2023

If you make it public, you also allow other places in the code to change the variable.

My preference would be to keep it protected and instead add an accessor function

  const Utilities::AsciiDataBoundary<dim> &get_slab_boundary() const;

This way, other places can access the data, but because they only get a const reference, they can't change it.

Separate question: Why was it protected to begin with, rather than private?

@alarshi
Copy link
Contributor Author

alarshi commented Jul 7, 2023

Thank you for the review! :) I addressed your comments in the latest commit.
I think Protected access instead of Private was a carryover mistake. I have corrected it now.

Copy link
Contributor

@bangerth bangerth left a comment

Choose a reason for hiding this comment

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

Yes, looks good. Thank you!

@bangerth bangerth merged commit 8392a78 into geodynamics:main Jul 8, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants