Le 27/07/2016 à 01:54, Scott Kostyshak a écrit :
The attached patch constifies a function parameter. My question is
whether this patch causes more pain to other developers than it does
good to the code.
The patch modifies a header that is included in many of our .cpp files,
so will cause a significant amount of recompilation.
I think I will come across this scenario in the future so I am curious:
what are your preferences?
(1) Do not commit any part of the patch because it is so minor.
(2) Commit the .cpp part, but not the .h part (this symmetry is not
checked by C++ since the constness only matters within the body of the
function).
(3) Commit the full patch, as is.
(4) Commit the full patch, as is. But save up these commits and push
only when I have a few of them or right after a commit that will cause
recompilation anyway.
Hi Scott,
- f(…, bool b)
+ f(…, bool const b)
This being passed by value, this changes nothing for the declaration.
The two signatures are even considered equal for overloading (if it
came to this). So I recommend leaving the header unchanged.
For the function definition, the difference is in terms of
documentation. If you are adding const in the definition because you
find it clearer this way, then it is a good reason to change it.
In this case, you can see "const" as a detail of implementation that the
user of the header does not care about. If you see it that way, then the
difference between the declaration and the definition looks strange no
longer.
The answer would be different if the const was somewhere inside, e.g.:
- f(…, bool & b)
+ f(…, bool const & b)
in which case the conveyed meaning is entirely different, and it would
be a matter of priority to correctly document the declaration.
Guillaume
