Pelvis moduel modification #29
Conversation
jilei-hao
left a comment
jilei-hao
left a comment
There was a problem hiding this comment.
Great work! @K-Prather
I especially like the way you added data into the project, simple and elegant. Modularization is also good, clean and effective coding.
| void Generate(); | ||
| vtkPolyData* GetOutput(); | ||
|
|
||
| private: |
There was a problem hiding this comment.
Internal data should be kept private and add public accessor such as GetXXXData(); SetXXXData(); for indirect public access.
| private: | ||
| float m_IntercristalDistance; | ||
| float m_PelvisScaleFactor; | ||
| vtkSmartPointer<vtkPolyData> m_PelvisOutputPolydata; |
There was a problem hiding this comment.
For data storage, a vtkSmartPointer is recommended rather than just a pointer. This brings convenience in memory management. Without this, an explicit deletion of this pointer should be included in the class' destructor, or there will be memory leak.
| PelvisModelGenerator(); | ||
| ~PelvisModelGenerator(); | ||
| PelvisModelGenerator(const PelvisModelGenerator &other); | ||
| PelvisModelGenerator(const PelvisModelGenerator &other) = delete; |
There was a problem hiding this comment.
It's great to always explicit define copy and assignment operators. But they does not always need to be enabled. We can define them to be deleted which prevent any copy or assignment operations, which are usually safer.
This case for a filter like class, unless we absolutely need it to be copied or assigned, we can define them to be deleted just to be safe.
|
|
||
| private: | ||
| float m_IntercristalDistance; | ||
| float m_PelvisScaleFactor; |
There was a problem hiding this comment.
The original name is absolutely fine. Using "m_" prefix for member variable is just a convention adopted by many people.
|
|
||
| this->intercristalDistance = 1; | ||
| this->pelvisScaleFactor = 1; | ||
| this->m_IntercristalDistance = 1; |
There was a problem hiding this comment.
In this file most of changes are variable name changes. See header file comment for the reason.
Pelvis module modification and review.