Should we move away from the practice that the member variables are initialized in the constructor, and rather move them to the declaration? At least for simple types. A “go to declaration” would also inform the programmer about member’s default value, not just its type and containing class.
Interesting. When initializing data members of a class this way (using C++11 in-class member initializers), it appears that their initial values are included automatically with the Doxygen generated documentation of the class! So they are self-documented!
So apparently, in-class member initialization reduces the chance of having mismatches between documentation and code (like this issue: Mismatch documentation and code SyNImageRegistrationMethod GaussianSmoothingVarianceForTheUpdateField default value).
From the outlined reasons, this looks a nice feature to me, if there are no downsides at least.
I guess not every data type may be initialized this way, but in any case, looks reasonable to consider it.
Also, and since this has arised a few times lately, if this somehow impacts performance vs using initialization lists due to some reason that I ignore right now, then we may need to be more cautious.
Finally, I guess the script using some regex to spot the initializations/assignment of member variables and move them to the declaration would not be that easy to develop. But may both spot uninitialized variables (much like using the
EXERCISE_BASIC_OBJECT_METHODS macro does in tests), as well as miss/identify others due to incorrect naming. So definitely a huge effort. But it may be a worthwhile one.
Thanks for your encouraging reply, Jon.
Maybe clang-tidy could do the job for us
Unfortunately I don’t have clang properly installed yet, on my Windows pc, and I’m not so familiar with clang anyway.
This might be a job for our resident clang-tidy master @hjmjohnson! Like he doesn’t have enough to do already
Indeed… I believe Hans can do it
In the mean time I did have another look. It appears that by default, the clang-tidy option modernize-use-default-member-init uses curly braces:
@dzenanz Is that OK to you as well? Otherwise the UseAssignment option has to be enabled.
Still I haven’t got clang-tidy-fix working properly here, especially passing this UseAssignment option seems non-obvious to me. So it would be great if Hans could do it.
Do curly braces appear in Doxygen as default values too? Otherwise I don’t have a preference.
Yes they do I just tried my local Doxygen installation (1.8.14), and it does display the initial values of the data members, in the generated HTML.
In general, curly braced initialization has more compile-time safety checking than the “assignment style” syntax. So maybe we should go for curly braces anyway.
Challenge accepted. Initial conversion is underway. PR to arrive shortly.
Great job, Hans! Thanks a lot!
P.S. It appears that the commit added a few unnecessary empty lines to constructor implementations: https://github.com/InsightSoftwareConsortium/ITK/pull/324/commits/a9a501e6cd7186671d5c47ab06ddc07d2cc126c7
Doesn’t matter much, of course. But could this be a clang-tidy bug?