STYLE: Replace postfix by prefix increment in `for` loops in Core/Common #2493

N-Dekker · 2021-04-19T16:13:57Z

Followed common C++ guidelines, including:

Google C++ Style Guide: "Use the prefix form (++i) of the increment and
decrement operators unless you need postfix semantics."
https://google.github.io/styleguide/cppguide.html#Preincrement_and_Predecrement

Herb Sutter, Andrei Alexandrescu - C++ Coding Standards: 101 Rules,
Guidelines, and Best Practices: "Prefer the canonical form of ++ and --.
Prefer calling the prefix forms"
https://www.oreilly.com/library/view/c-coding-standards/0321113586/ch29.html

Applied at GNU bash, by the following command:

find . \( -iname *.cxx -or -iname *.hxx -or -iname *.h \) -exec perl -pi -w -e 's/(\s+for \(.*; .+); (\w+)\+\+\)/$1; ++$2)/g;' {} \;

Manually replaced a few more postfix increments, which were not detected
by this regex command, as remarked by Mikhail Isakov (@issakomi).

dzenanz · 2021-04-19T16:35:09Z

Modules/Core/Common/include/itkArray.h

@@ -104,7 +104,7 @@ class ITK_TEMPLATE_EXPORT Array : public vnl_vector<TValue>
  {
    this->m_LetArrayManageMemory = true;
    this->SetSize(r.GetSize());
-    for (SizeValueType i = 0; i < r.GetSize(); i++)
+    for (SizeValueType i = 0; i < r.GetSize(); ++i)


Ahhh, ++i looks so strange!

Ahhh, ++i looks so strange!

@dzenanz Are you serious? We could introduce a function, itk::Increment(i)... 😸

N-Dekker · 2021-04-19T16:54:10Z

FYI, it looks like a Clang-Tidy option to maintain this style is underway:
"[clang-tidy] Add performance-prefer-preincrement check"
https://reviews.llvm.org/D72553

dpshamonin · 2021-04-20T10:38:15Z

👍

issakomi · 2021-04-20T16:35:10Z

Modules/Core/Common/test/itkNeighborhoodIteratorTest.cxx

@@ -83,7 +83,7 @@ itkNeighborhoodIteratorTest(int, char *[])

  it3.Print(std::cout);
  unsigned x, y, i;
-  for (y = 0, i = 0; y < 5; y++)
+  for (y = 0, i = 0; y < 5; ++y)
  {
    for (x = 0; x < 5; x++, i++)


FYI, here postfix increment was not detected by regex, probably, (and several below the line)

Probably all multi-variable increments are missed.

Good catch @issakomi Do you have a suggestion how to catch and fix such cases by regex?

Obviously we must be 100% sure that the semantics don't change by the regex!

Sorry, not yet sure how to automate this in reliable way. Looks like in Core/Common multi-variable for with postfix increment are below, at least with "for" and "," at a single line

./test/itkNeighborhoodIteratorTest.cxx: for (x = 0; x < 5; x++, i++) ./test/itkNeighborhoodIteratorTest.cxx: for (x = 0; x < 5; x++, i++) ./test/itkNeighborhoodIteratorTest.cxx: for (x = 0; x < 5; x++, i++) ./test/itkNeighborhoodIteratorTest.cxx: for (x = 0; x < 5; x++, i++) ./test/itkNeighborhoodIteratorTest.cxx: for (x = 0; x < 5; x++, i++) ./test/itkNeighborhoodIteratorTest.cxx: for (x = 0; x < 5; x++, i++) ./test/itkNeighborhoodIteratorTest.cxx: for (x = 0; x < 5; x++, i++) ./test/itkNeighborhoodIteratorTest.cxx: for (x = 0; x < 5; x++, i++) ./test/itkNeighborhoodIteratorTest.cxx: for (x = 0; x < 5; x++, i++) ./test/itkObjectFactoryTest.cxx: for (std::list<std::string>::const_iterator o = overrides.begin(); o != overrides.end(); ++o, ++n, ++d, e++) ./test/itkPriorityQueueTest.cxx: for (; it != sequence.end(); ++it, i++) ./test/itkObjectFactoryTest2.cxx: for (std::list<std::string>::const_iterator o = overrides.begin(); o != overrides.end(); ++o, ++n, ++d, e++) ./include/itkNeighborhoodIterator.hxx: for (N_it = N.Begin(), this_it = this->Begin(); this_it < _end; this_it++, N_it++) ./include/itkNeighborhoodIterator.hxx: for (N_it = N.Begin(), this_it = this->Begin(); this_it < _end; this_it++, N_it++) ./include/itkNeighborhoodIterator.hxx: for (N_it = N.Begin(), this_it = this->Begin(); this_it < _end; N_it++, this_it++)

We don't have to change every instance now. Getting most of them is sufficient improvement.

Please check my latest force-push 😄 https://github.com/InsightSoftwareConsortium/ITK/compare/ff9e1a7584564ff6a54b5191277847793ffe9fe4..d7eea8a76120648be23474c379894ef0338878f0

N-Dekker · 2021-04-20T19:17:46Z

Back to "work in progress": an update is underway!

Followed common C++ guidelines, including: Google C++ Style Guide: "Use the prefix form (++i) of the increment and decrement operators unless you need postfix semantics." https://google.github.io/styleguide/cppguide.html#Preincrement_and_Predecrement Herb Sutter, Andrei Alexandrescu - C++ Coding Standards: 101 Rules, Guidelines, and Best Practices: "Prefer the canonical form of ++ and --. Prefer calling the prefix forms" https://www.oreilly.com/library/view/c-coding-standards/0321113586/ch29.html Applied at GNU bash, by the following command: find . $ -iname *.cxx -or -iname *.hxx -or -iname *.h $ -exec perl -pi -w -e 's/(\s+for $.*; .+); (\w+)\+\+$/$1; ++$2)/g;' {} \; Manually replaced a few more postfix increments, which were not detected by this regex command, as remarked by Mikhail Isakov (issakomi).

Applied to ITK/Modules, at the GNU bash prompt: find . $ -iname *.cxx -or -iname *.hxx -or -iname *.h $ -exec perl -pi -w -e 's/(\s+for $.*; .+); (\w+)\+\+$/$1; ++$2)/g;' {} \; (Excluded ITK/Modules/ThirdParty.) Follow-up to: pull request InsightSoftwareConsortium#2493 commit f81578b "STYLE: Replace postfix by prefix increment in `for` loops in Core/Common"

Applied to ITK/Modules, at the GNU bash prompt: find . $ -iname *.cxx -or -iname *.hxx -or -iname *.h $ -exec perl -pi -w -e 's/(\s+for $.*; .+); (\w+)\+\+$/$1; ++$2)/g;' {} \; (Excluded ITK/Modules/ThirdParty.) According to Herb Sutter, Andrei Alexandrescu - C++ Coding Standards: 101 Rules, Guidelines, and Best Practices: "Prefer the canonical form of ++ and --. Prefer calling the prefix forms" Follow-up to: pull request InsightSoftwareConsortium#2493 commit f81578b "STYLE: Replace postfix by prefix increment in `for` loops in Core/Common"

Applied to ITK/Modules, at the GNU bash prompt: find . $ -iname *.cxx -or -iname *.hxx -or -iname *.h $ -exec perl -pi -w -e 's/(\s+for $.*; .+); (\w+)\+\+$/$1; ++$2)/g;' {} \; (Excluded ITK/Modules/ThirdParty.) According to Herb Sutter, Andrei Alexandrescu - C++ Coding Standards: 101 Rules, Guidelines, and Best Practices: "Prefer the canonical form of ++ and --. Prefer calling the prefix forms" Follow-up to: pull request #2493 commit f81578b "STYLE: Replace postfix by prefix increment in `for` loops in Core/Common"

Follows common C++ guidelines, including: Google C++ Style Guide: "Use the prefix form (++i) of the increment and decrement operators unless you need postfix semantics." https://google.github.io/styleguide/cppguide.html#Preincrement_and_Predecrement Herb Sutter, Andrei Alexandrescu - C++ Coding Standards: 101 Rules, Guidelines, and Best Practices: "Prefer the canonical form of ++ and --. Prefer calling the prefix forms" https://www.oreilly.com/library/view/c-coding-standards/0321113586/ch29.html Anticipates "[clang-tidy] Add performance-prefer-preincrement check": https://reviews.llvm.org/D72553 Corresponding ITK pull requests, which were merged in April 2021: - InsightSoftwareConsortium/ITK#2505 - InsightSoftwareConsortium/ITK#2499 - InsightSoftwareConsortium/ITK#2493

Applied to the root of elastix source tree, at the GNU bash prompt: $ find . $ -iname *.cxx -or -iname *.hxx -or -iname *.h $ -exec perl -pi -w -e 's/(\s+for $.*;.*); (\w+)\+\+$/$1; ++$2)/g;' {} \; Follows common C++ guidelines, including: Google C++ Style Guide: "Use the prefix form (++i) of the increment and decrement operators unless you need postfix semantics." https://google.github.io/styleguide/cppguide.html#Preincrement_and_Predecrement Herb Sutter, Andrei Alexandrescu - C++ Coding Standards: 101 Rules, Guidelines, and Best Practices: "Prefer the canonical form of ++ and --. Prefer calling the prefix forms" https://www.oreilly.com/library/view/c-coding-standards/0321113586/ch29.html Anticipates "[clang-tidy] Add performance-prefer-preincrement check": https://reviews.llvm.org/D72553 Corresponding ITK pull requests, which were merged in April 2021: - InsightSoftwareConsortium/ITK#2505 - InsightSoftwareConsortium/ITK#2499 - InsightSoftwareConsortium/ITK#2493

Applied to the root of ITK source tree, at the GNU bash prompt: find . $ -iname itk*.cxx -or -iname itk*.hxx -or -iname itk*.h $ -exec perl -pi -w -e 's/ (\w+)\+\+;/ ++$1;/g;' {} \; Follow-up to: pull request InsightSoftwareConsortium#2493 commit f81578b "STYLE: Replace postfix by prefix increment in `for` loops in Core/Common"

Applied to the root of ITK source tree, at the GNU bash prompt: find . $ -iname itk*.cxx -or -iname itk*.hxx -or -iname itk*.h $ -exec perl -pi -w -e 's/ (\w+)\+\+;/ ++$1;/g;' {} \; find . $ -iname itk*.cxx -or -iname itk*.hxx -or -iname itk*.h $ -exec perl -pi -w -e 's/ (\w+)\-\-;/ --$1;/g;' {} \; Follow-up to: pull request InsightSoftwareConsortium#2493 commit f81578b "STYLE: Replace postfix by prefix increment in `for` loops in Core/Common"

Applied to the root of ITK source tree, at the GNU bash prompt: find . $ -iname itk*.cxx -or -iname itk*.hxx -or -iname itk*.h $ -exec perl -pi -w -e 's/ (\w+)\+\+;/ ++$1;/g;' {} \; find . $ -iname itk*.cxx -or -iname itk*.hxx -or -iname itk*.h $ -exec perl -pi -w -e 's/ (\w+)\-\-;/ --$1;/g;' {} \; Excluded itkConceptChecking.h, as it is intended to check both the prefix *and* the postfix forms. Excluded the unit tests, as they might intentionally test the postfix forms. Follow-up to: pull request InsightSoftwareConsortium#2493 commit f81578b "STYLE: Replace postfix by prefix increment in `for` loops in Core/Common"

Applied to the root of ITK source tree, at the GNU bash prompt: find . $ -iname itk*.cxx -or -iname itk*.hxx -or -iname itk*.h $ -exec perl -pi -w -e 's/ (\w+)\+\+;/ ++$1;/g;' {} \; find . $ -iname itk*.cxx -or -iname itk*.hxx -or -iname itk*.h $ -exec perl -pi -w -e 's/ (\w+)\-\-;/ --$1;/g;' {} \; Excluded itkConceptChecking.h, as it is intended to check both the prefix *and* the postfix forms. Excluded the unit tests, as they might intentionally test the postfix forms. Follow-up to: pull request #2493 commit f81578b "STYLE: Replace postfix by prefix increment in `for` loops in Core/Common"

dzenanz approved these changes Apr 19, 2021

View reviewed changes

N-Dekker marked this pull request as ready for review April 19, 2021 20:11

dpshamonin approved these changes Apr 20, 2021

View reviewed changes

hjmjohnson approved these changes Apr 20, 2021

View reviewed changes

issakomi reviewed Apr 20, 2021

View reviewed changes

N-Dekker marked this pull request as draft April 20, 2021 19:00

N-Dekker force-pushed the prefix-increment branch from dff7824 to ff9e1a7 Compare April 20, 2021 20:17

N-Dekker force-pushed the prefix-increment branch from ff9e1a7 to d7eea8a Compare April 20, 2021 20:23

N-Dekker marked this pull request as ready for review April 20, 2021 20:24

dzenanz approved these changes Apr 20, 2021

View reviewed changes

N-Dekker merged commit f81578b into InsightSoftwareConsortium:master Apr 21, 2021

N-Dekker mentioned this pull request Apr 21, 2021

STYLE: Replace postfix by prefix increment in for loops in ITK/Modules #2499

Merged

N-Dekker mentioned this pull request Apr 23, 2021

STYLE: Replace postfix by prefix increment in for loops in Examples #2505

Merged

N-Dekker mentioned this pull request May 20, 2021

STYLE: Replace postfix by prefix increment in for loops SuperElastix/elastix#467

Merged

N-Dekker mentioned this pull request Jul 11, 2022

Replace postfix with prefix increment and decrement #3484

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STYLE: Replace postfix by prefix increment in `for` loops in Core/Common #2493

STYLE: Replace postfix by prefix increment in `for` loops in Core/Common #2493

N-Dekker commented Apr 19, 2021 •

edited

Loading

dzenanz Apr 19, 2021

N-Dekker Apr 19, 2021

N-Dekker commented Apr 19, 2021

dpshamonin commented Apr 20, 2021

issakomi Apr 20, 2021 •

edited

Loading

dzenanz Apr 20, 2021

N-Dekker Apr 20, 2021

issakomi Apr 20, 2021

dzenanz Apr 20, 2021

N-Dekker Apr 20, 2021

N-Dekker commented Apr 20, 2021

STYLE: Replace postfix by prefix increment in for loops in Core/Common #2493

STYLE: Replace postfix by prefix increment in for loops in Core/Common #2493

Conversation

N-Dekker commented Apr 19, 2021 • edited Loading

dzenanz Apr 19, 2021

Choose a reason for hiding this comment

N-Dekker Apr 19, 2021

Choose a reason for hiding this comment

N-Dekker commented Apr 19, 2021

dpshamonin commented Apr 20, 2021

issakomi Apr 20, 2021 • edited Loading

Choose a reason for hiding this comment

dzenanz Apr 20, 2021

Choose a reason for hiding this comment

N-Dekker Apr 20, 2021

Choose a reason for hiding this comment

issakomi Apr 20, 2021

Choose a reason for hiding this comment

dzenanz Apr 20, 2021

Choose a reason for hiding this comment

N-Dekker Apr 20, 2021

Choose a reason for hiding this comment

N-Dekker commented Apr 20, 2021

STYLE: Replace postfix by prefix increment in `for` loops in Core/Common #2493

STYLE: Replace postfix by prefix increment in `for` loops in Core/Common #2493

N-Dekker commented Apr 19, 2021 •

edited

Loading

issakomi Apr 20, 2021 •

edited

Loading