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.

Sign up for GitHub

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

STYLE: Call SetNthOutput(i, MakeOutput(i)) in a for loop #4687

Closed
wants to merge 1 commit into from
Closed

STYLE: Call SetNthOutput(i, MakeOutput(i)) in a for loop #4687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions Modules/Filtering/DistanceMap/include/itkDanielssonDistanceMapImageFilter.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,14 @@ namespace itk
template <typename TInputImage, typename TOutputImage, typename TVoronoiImage>
DanielssonDistanceMapImageFilter<TInputImage, TOutputImage, TVoronoiImage>::DanielssonDistanceMapImageFilter()
{
this->SetNumberOfRequiredOutputs(3);
constexpr unsigned int numberOfRequiredOutputs{ 3 };
this->SetNumberOfRequiredOutputs(numberOfRequiredOutputs);

// distance map
this->SetNthOutput(0, this->MakeOutput(0));

// voronoi map
this->SetNthOutput(1, this->MakeOutput(1));

// distance vectors
this->SetNthOutput(2, this->MakeOutput(2));
// distance map, voronoi map, distance vectors
for (unsigned int i{}; i < numberOfRequiredOutputs; ++i)
{
ProcessObject::SetNthOutput(i, Self::MakeOutput(i));
}
Comment on lines +36 to +40
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we can make this more generic, like:

ProcessObject::SetRequiredOutputs(*this, numberOfRequiredOutputs);

This new function, SetRequiredOutputs would then set the numberOfRequiredOutputs and do the for loop. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

For the constructors we are in a bit of a hard place where the methods called be the constructor should not be virtual. That is the virtual MakeOutput should not be called by base classes in the constructor.

This is more of a design issue, maybe the output should only be created on demand? or Maybe a separate "virtual Initialize" method is needed.

Then there is the idea of having a base class take multiple template parameters for inputs and outputs.... But for the few filters it's likely not worth the refactor.


m_SquaredDistance = false;
m_InputIsBinary = false;
Expand Down
16 changes: 7 additions & 9 deletions Modules/Filtering/DistanceMap/include/itkSignedDanielssonDistanceMapImageFilter.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,14 @@ template <typename TInputImage, typename TOutputImage, typename TVoronoiImage>
SignedDanielssonDistanceMapImageFilter<TInputImage, TOutputImage, TVoronoiImage>::
SignedDanielssonDistanceMapImageFilter()
{
this->SetNumberOfRequiredOutputs(3);
constexpr unsigned int numberOfRequiredOutputs{ 3 };
this->SetNumberOfRequiredOutputs(numberOfRequiredOutputs);

// distance map
this->SetNthOutput(0, static_cast<OutputImageType *>(this->MakeOutput(0).GetPointer()));

// voronoi map
this->SetNthOutput(1, static_cast<VoronoiImageType *>(this->MakeOutput(1).GetPointer()));

// distance vectors
this->SetNthOutput(2, static_cast<VectorImageType *>(this->MakeOutput(2).GetPointer()));
// distance map, voronoi map, distance vectors
for (unsigned int i{}; i < numberOfRequiredOutputs; ++i)
{
ProcessObject::SetNthOutput(i, Self::MakeOutput(i));
}

// Default values
this->m_SquaredDistance = false; // Should we remove this ?
Expand Down
10 changes: 6 additions & 4 deletions Modules/Numerics/Eigen/include/itkEigenAnalysis2DImageFilter.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@ namespace itk
template <typename TInputImage, typename TEigenValueImage, typename TEigenVectorImage>
EigenAnalysis2DImageFilter<TInputImage, TEigenValueImage, TEigenVectorImage>::EigenAnalysis2DImageFilter()
{
constexpr unsigned int numberOfRequiredOutputs{ 3 };
this->SetNumberOfRequiredInputs(3);
this->SetNumberOfRequiredOutputs(3);
this->SetNthOutput(0, this->MakeOutput(0));
this->SetNthOutput(1, this->MakeOutput(1));
this->SetNthOutput(2, this->MakeOutput(2));
this->SetNumberOfRequiredOutputs(numberOfRequiredOutputs);
for (unsigned int i{}; i < numberOfRequiredOutputs; ++i)
{
ProcessObject::SetNthOutput(i, Self::MakeOutput(i));
}
static_assert(EigenVectorType::Dimension == 2, "Error: PixelType of EigenVector Image must have exactly 2 elements!");
}

Expand Down
17 changes: 10 additions & 7 deletions Modules/Segmentation/Watersheds/include/itkWatershedSegmenter.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1354,13 +1354,16 @@ Segmenter<TInputImage>::Segmenter()
m_SortEdgeLists = true;
m_Connectivity.direction = nullptr;
m_Connectivity.index = nullptr;
typename OutputImageType::Pointer img = static_cast<OutputImageType *>(this->MakeOutput(0).GetPointer());
typename SegmentTableType::Pointer st = static_cast<SegmentTableType *>(this->MakeOutput(1).GetPointer());
typename BoundaryType::Pointer bd = static_cast<BoundaryType *>(this->MakeOutput(2).GetPointer());
this->SetNumberOfRequiredOutputs(3);
this->ProcessObject::SetNthOutput(0, img.GetPointer());
this->ProcessObject::SetNthOutput(1, st.GetPointer());
this->ProcessObject::SetNthOutput(2, bd.GetPointer());

constexpr unsigned int numberOfRequiredOutputs{ 3 };
this->SetNumberOfRequiredOutputs(numberOfRequiredOutputs);

// OutputImage, SegmentTable, Boundary
for (unsigned int i{}; i < numberOfRequiredOutputs; ++i)
{
ProcessObject::SetNthOutput(i, Self::MakeOutput(i));
}


// Allocate memory for connectivity
m_Connectivity.size = 2 * ImageDimension;
Expand Down