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

Improved style of m_ComputePerThreadVariable data members #132

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

N-Dekker
Copy link
Member

No description provided.

@N-Dekker
Copy link
Member Author

Still need to fix this error, before merging:

common\itkComputePreconditionerUsingDisplacementDistribution.hxx(346): error C2248: 'itk::ComputeDisplacementDistribution<TFixedImage,TTransform>::m_Transform': cannot access private member declared in class 'itk::ComputeDisplacementDistribution<TFixedImage,TTransform>' 

@N-Dekker N-Dekker force-pushed the STYLE-ComputePerThreadVariables branch 2 times, most recently from d089c1e to 9e6ca5a Compare March 31, 2019 22:08
Made data members of `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` private, including their `m_ComputePerThreadVariables` member.
Declared m_ComputePerThreadVariables of both as `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` as an `std::vector<AlignedComputePerThreadStruct>`, instead of a raw pointer to the structs. Simplified zero-initialization of the structs.

Removed manual `delete[]` statements.

This commit aims to improve modern C++ style, and avoid memory leaks.
Removed the `mutable` keyword from `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` data members.

The `mutable` keyword should only be used when really necessary (specifically for const-correctness).
@mstaring
Copy link
Member

mstaring commented Apr 1, 2019

this is actually a candidate class that can be moved to the new ITK5 threading mechanism, at it is quite stand alone. Could you have a stab Niels?

Made data members of `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` private, including their `m_ComputePerThreadVariables` member.
Declared m_ComputePerThreadVariables of both as `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` as an `std::vector<AlignedComputePerThreadStruct>`, instead of a raw pointer to the structs. Simplified zero-initialization of the structs.

Removed manual `delete[]` statements.

This commit aims to improve modern C++ style, and avoid memory leaks.
Removed the `mutable` keyword from `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` data members.

The `mutable` keyword should only be used when really necessary (specifically for const-correctness).
@N-Dekker
Copy link
Member Author

Just did git rebase develop, git pull, git push, hoping to fix the compile errors.

N-Dekker added a commit that referenced this pull request May 5, 2022
Made data members of `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` private, including their `m_ComputePerThreadVariables` member.

Based on the first commit of pull request #132 "Improved style of m_ComputePerThreadVariable data members".

Follows C++ Core Guidelines (April 10, 2022): "C.133: Avoid protected data" http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-protected
N-Dekker added a commit that referenced this pull request May 5, 2022
Made data members of `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` private, including their `m_ComputePerThreadVariables` member.

Based on the first commit of pull request #132 "Improved style of m_ComputePerThreadVariable data members".

Follows C++ Core Guidelines (April 10, 2022): "C.133: Avoid protected data" http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-protected
N-Dekker added a commit that referenced this pull request May 5, 2022
Declared m_ComputePerThreadVariables of both as `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` as an `std::vector<AlignedComputePerThreadStruct>`, instead of a raw pointer to the structs. Simplified zero-initialization of the structs. Removed manual `delete[]` statements.

Based on the second commit of pull request #132 "Improved style of m_ComputePerThreadVariable data members".

Follows C++ Core Guidelines (April 10, 2022): "Avoid calling new and delete explicitly" http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-ptr
N-Dekker added a commit that referenced this pull request May 5, 2022
Declared m_ComputePerThreadVariables of both as `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` as an `std::vector<AlignedComputePerThreadStruct>`, instead of a raw pointer to the structs. Simplified zero-initialization of the structs. Removed manual `delete[]` statements.

Based on the second commit of pull request #132 "Improved style of m_ComputePerThreadVariable data members".

Follows C++ Core Guidelines (April 10, 2022): "Avoid calling new and delete explicitly" http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-ptr
N-Dekker added a commit that referenced this pull request May 6, 2022
Declared m_ComputePerThreadVariables of both as `AdvancedImageMomentsCalculator` and `ComputeDisplacementDistribution` as an `std::vector<AlignedComputePerThreadStruct>`, instead of a raw pointer to the structs. Simplified zero-initialization of the structs. Removed manual `delete[]` statements.

Based on the second commit of pull request #132 "Improved style of m_ComputePerThreadVariable data members".

Follows C++ Core Guidelines (April 10, 2022): "Avoid calling new and delete explicitly" http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-ptr
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