search

InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.4k stars 662 forks source link

STYLE: Remove ` == true` from Boolean expressions #4502

Closed N-Dekker closed 5 months ago

N-Dekker commented 5 months ago

Following C++ Core Guidelines, February 15, 2024, "Don’t add redundant == or != to conditions", http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es87-dont-add-redundant--or--to-conditions

thewtex commented 5 months ago

Windows failure:

The following tests FAILED:
    779 - itkConvolutionImageFilterStreamingValidTest (Failed)

test flake?

N-Dekker commented 5 months ago

/azp run ITK.Windows

N-Dekker commented 5 months ago

Did you use some tool to find instances of this?

Well, I did it step by step, "heuristically", starting with a global search of just m_UseImageSpacing == true, then using regular expressions, ( if \(\w+) == true\), ( if \([^ ]+) == true\), normal search == true), and == true ||, etc. Until I felt most relevant cases were addressed.

Looking back, I could have done just a brute force find == true and replace with (nothing). There are only a few == true cases left now. Not sure if I should also address them.

Not yet addressed:

* \post `m_LetArrayManageMemory == true`

https://github.com/InsightSoftwareConsortium/ITK/blob/e3189492ce6858035b68a2b89e4a4d2f1d4041ad/Modules/Core/Common/include/itkVariableLengthVector.h#L442

N-Dekker commented 5 months ago

Also not addressed:

https://github.com/InsightSoftwareConsortium/ITK/blob/e3189492ce6858035b68a2b89e4a4d2f1d4041ad/Modules/Filtering/DistanceMap/test/itkSignedDanielssonDistanceMapImageFilterTest11.cxx#L136

N-Dekker commented 5 months ago

This one is not addressed, but maybe it should, what do you think?

https://github.com/InsightSoftwareConsortium/ITK/blob/e3189492ce6858035b68a2b89e4a4d2f1d4041ad/Modules/Core/Common/test/itkMetaProgrammingLibraryTest.cxx#L30

dzenanz commented 5 months ago

Yes, you could also update that.

N-Dekker commented 5 months ago

FYI While I think this pull request is very much in line with C++ Core Guidelines item "Don’t add redundant == or != to conditions", I believe it would be helpful if the guideline would explicitly mention such b == true cases. So I also submitted a pull request to the C++ Core Guidelines 😃 :