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

STYLE: Replace Fill calls with auto var = itk::MakeFilled<T> in tests #4891

Conversation

N-Dekker
Copy link
Contributor

Replaced code of the form

T var;
var.Fill(x);

with auto var = itk::MakeFilled<T>(x);

Following C++ Core Guidelines, Oct 3, 2024, "Always initialize an object", https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-always

Using Notepad++, Replace in Files, doing:

Find what: ^( [ ]+)([^ ].*)[ ]+(\w+);[\r\n]+\1\3\.Fill\(
Replace with: $1auto $3 = itk::MakeFilled<$2>\(
Filters: itk*Test*.cxx
[v] Match case
(*) Regular expression

Follow-up to

@github-actions github-actions bot added type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module type:Style Style changes: no logic impact (indentation, comments, naming) area:Numerics Issues affecting the Numerics module labels Oct 22, 2024
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Looks good on a glance.

@N-Dekker
Copy link
Contributor Author

Honestly I find the syntax auto var = itk::MakeFilled<T>(x) a bit verbose. For zero-filling I would certainly prefer the syntax T var{}. But I still think auto var = itk::MakeFilled<T>(x) is very much preferable to declaring the variable uninitialized and calling Fill afterwards.

As a follow-up, the variable might in some cases be declared constexpr:

    // Assuming that the fill value x is also a compile-time constant:
    constexpr auto var = itk::MakeFilled<T>(x); 

😃

@N-Dekker
Copy link
Contributor Author

N-Dekker commented Oct 22, 2024

Hmmm, compilers don't like itk::MakeFilled for an itk::Matrix. 🤔 To be continued...


Let's postpone this pull request until after the following two have been processed:

Replaced code of the form

    T var;
    var.Fill(x);

with `auto var = itk::MakeFilled<T>(x);`

Following C++ Core Guidelines, Oct 3, 2024, "Always initialize an object",
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-always

Using Notepad++, Replace in Files, doing:

    Find what: ^( [ ]+)([^ ].*)[ ]+(\w+);[\r\n]+\1\3\.Fill\(
    Replace with: $1auto $3 = itk::MakeFilled<$2>\(
    Filters: itk*Test*.cxx
    [v] Match case
    (*) Regular expression

Follow-up to
- pull request InsightSoftwareConsortium#4881
- pull request InsightSoftwareConsortium#4884
- pull request InsightSoftwareConsortium#4887
@N-Dekker N-Dekker force-pushed the Replace-Fill-with-itk-MakeFilled-in-tests branch from 7750331 to d6c9eed Compare October 25, 2024 21:36
@N-Dekker N-Dekker marked this pull request as ready for review October 26, 2024 08:41
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Oct 26, 2024
Replaced code of the form

    T var;
    var.Fill(x);

with `auto var = itk::MakeFilled<T>(x);`

- Follow-up to pull request InsightSoftwareConsortium#4891
"STYLE: Replace Fill calls with `auto var = itk::MakeFilled<T>` in tests"
@jhlegarreta jhlegarreta merged commit 4b7886a into InsightSoftwareConsortium:master Oct 27, 2024
13 checks passed
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Oct 31, 2024
Using Notepad++, Replace in Files, doing:

    Find what: ^( [ ]+)([^ ].*)[ ]+(\w+);[\r\n]+\1\3\.Fill\(
    Replace with: $1auto $3 = MakeFilled<$2>\(
    Filters: itk*.h;itk*.hxx;itk*.cxx
    Directory: D:\src\ITK\Modules
    [v] Match case
    (*) Regular expression

Excluded "itkVectorMeanImageFunction.hxx", because it may try to fill a
`VariableLengthVector` (which is not supported by `MakeFilled`).

Follow-up to pull request InsightSoftwareConsortium#4891
commit d6c9eed
"STYLE: Replace Fill calls with `auto var = itk::MakeFilled<T>` in tests"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 1, 2024
Using Notepad++, Replace in Files, doing:

    Find what: ^( [ ]+)([^ ].*)[ ]+(\w+);[\r\n]+\1\3\.Fill\(
    Replace with: $1auto $3 = MakeFilled<$2>\(
    Filters: itk*.h;itk*.hxx;itk*.cxx
    Directory: D:\src\ITK\Modules
    [v] Match case
    (*) Regular expression

Excluded "itkVectorMeanImageFunction.hxx", because it may try to fill a
`VariableLengthVector` (which is not supported by `MakeFilled`).

Follow-up to pull request InsightSoftwareConsortium#4891
commit d6c9eed
"STYLE: Replace Fill calls with `auto var = itk::MakeFilled<T>` in tests"
dzenanz pushed a commit that referenced this pull request Nov 4, 2024
Using Notepad++, Replace in Files, doing:

    Find what: ^( [ ]+)([^ ].*)[ ]+(\w+);[\r\n]+\1\3\.Fill\(
    Replace with: $1auto $3 = MakeFilled<$2>\(
    Filters: itk*.h;itk*.hxx;itk*.cxx
    Directory: D:\src\ITK\Modules
    [v] Match case
    (*) Regular expression

Excluded "itkVectorMeanImageFunction.hxx", because it may try to fill a
`VariableLengthVector` (which is not supported by `MakeFilled`).

Follow-up to pull request #4891
commit d6c9eed
"STYLE: Replace Fill calls with `auto var = itk::MakeFilled<T>` in tests"
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Nov 15, 2024
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Numerics Issues affecting the Numerics module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants