STYLE: Use itkGetConstMacro for getting 1D FFT direction

[tbirdso]

Prompted by discussion in #2805.

PR Checklist

• No API changes were made (or the changes have been approved)
• No major design changes were made (or the changes have been approved)

Refer to the ITK Software Guide for
further development details if necessary.

[jhlegarreta]

A deprecation cycle is needed, isn't it? Or else we assume that it is closer to an undesired behavior, bug and no such cycle applies? Tagging @N-Dekker.

[N-Dekker]

@jhlegarreta

A deprecation cycle is needed

Do you really think so? A Get member function defined by itkGetConstMacro supports getting a value from a const object, as well as from a non-const object.

[jhlegarreta]

Do you really think so?

Not sure.

[dzenanz]

The code which used ITKUltrasound would need to be updated. The prinicipally involved persons, @thewtex and @tbirdso, should decide whether they want to update it now or later. If they want to update now, deprecation cycle is not needed because this code was just introduced in ITK proper and a release with it has not been made yet.

[tbirdso]

@dzenanz Sorry, I'm not sure what is being asked. Is the question in regards to updating code in the ITKUltrasound module that may be using GetDirection from these 1D FFT filters as a non-const member and therefore breaking the new API? Or is it concern for consumers in ITK proper? It doesn't look like GetDirection is referenced in 1D FFT tests other than the standard ITK_EXERCISE_BASIC_OBJECT_METHODS macro.

[dzenanz]

I meant application code which used 1D FFTs from ITKUltrasound. That will need to be updated.

[tbirdso]

Thanks. Maybe @thewtex can weigh in the merit of the change + applications potentially affected?

[jhlegarreta]

deprecation cycle is not needed because this code was just introduced in ITK proper and a release with it has not been made yet.

Very true. Thanks @dzenanz 👌 !!

[N-Dekker]

I meant application code which used 1D FFTs from ITKUltrasound. That will need to be updated.

This pull request does not break any existing filter.GetDirection() call, as far as I can see. itkGetConstMacro(Direction, unsigned int) supports getting the direction from both a const and a non-const filter. It would only break user code that does override GetDirection(). But in general, a Get function that just yields the value of a data member is rarely overridden.

My two cents. 😃

[N-Dekker]

@tbirdso By the way, why did you use the "PERF" prefix? I wouldn't directly expect a significant performance improvement when switching from itkGetMacro to itkGetConstMacro, looking at https://github.com/InsightSoftwareConsortium/ITK/blob/v5.3rc01/Modules/Core/Common/include/itkMacro.h#L964-L974 So I think I would use the "STYLE" prefix instead.

[thewtex]

To clarify, @jhlegarreta is right to keep an eye on backwards compatibility, but these classes are being introduced in ITK from the ITKUltrasound module in ITK 5.3 (still in the RC stage), so no deprecation is needed. Also, as @N-Dekker , there should be not be changes required in client code. @tbirdso thanks for the follow-up on this. Please reclassify the patch as STYLE as @N-Dekker suggests, and it should be ready to go!

[tbirdso]

Fixed. Thanks everyone for the discussion on this!