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

[N-Dekker]

Reduced duplicate code by replacing manual SetNthOutput(i, MakeOutput(i)) calls for i = 0, 1, 2 with the corresponding for loops.

[N-Dekker]

@blowekamp Following our discussion at #4654 (comment) I'm still trying to get clear in which particular scenario's MakeOutput(i) is actually useful, within a default-constructor. Is this an example use case? I mean, the ability to call it iteratively in a for loop?

[N-Dekker]

OK, so this could be a nice use case, being able to call Self::MakeOutput(i) is a for loop. Still I'm interested to hear if calling Self::MakeOutput(i) is also useful outside the constructor of a class (the class Self).

[blowekamp]

@blowekamp Following our discussion at #4654 (comment) I'm still trying to get clear in which particular scenario's MakeOutput(i) is actually useful, within a default-constructor. Is this an example use case? I mean, the ability to call it iteratively in a for loop?

Thanks for the continued investigation and the discussion. These filters have multiple outputs of different types. These different output types are not represented by the template parameters. Hence the MakeOutput method has been overloaded to create the correct "nth" output. This is important because the base class ImageSource just creates all outputs of the same type. Having the correctly typed output was a hard issue to track down with forced casted and failed dynamic casts. The pattern used in as many places possible was to use the MakeOutput method as the generic way to create the outputs.

Note: An incorrectly casted image type can work a surprising long time as the wrong type.

[blowekamp]

Still I'm interested to hear if calling Self::MakeOutput(i) is also useful outside the constructor of a class (the class Self).

It is also used here:

ITK/Modules/Core/Common/src/itkProcessObject.cxx

Line 464 in 8e5abbd

DataObjectPointer newOutput = this->MakeOutput(key);

Which indirectly gets called when operations the "ReleaseDataFlagOn" or "DisconnectPipeline" are called and the output created in the constructor is created again.

[N-Dekker]

It is also used here:

ITK/Modules/Core/Common/src/itkProcessObject.cxx

Line 464 in 8e5abbd

DataObjectPointer newOutput = this->MakeOutput(key);

Interesting, thanks! Apparently SetOutput(name, output) creates a new output by calling MakeOutput, if the output argument passed by the user is NULL. I did not know that. 🤷

[N-Dekker]

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?

[blowekamp]

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.

[N-Dekker]

• Closed (not merged), as it is superseded by pull request Add and use ProcessObject::MakeRequiredOutputs #4688