On 06/08/2016 17:04, Guillaume Munch wrote:
Le 06/08/2016 à 10:16, Abdelrazak Younes a écrit :
(1) Do not commit any part of the patch because it is so minor.
This is my preference because it doesn't bring anything and it can
create confusion for the beginner when compared with const
reference.
But I am sceptical of a code guideline that discourages documenting
the code (for instance with const) when one feels that it is appropriate.
I'd rather this guy take the time to write down a nice doxygen comment
in the header to explain the parameter.
The C++ beginner could be tempted to add a "&" here...
If a beginner changes const into const & without understanding what they
are doing, then this hints at an issue which is different from
inexperience. I have to reject this argument.
Shit happens :-) And this kind of mistake are easily missed even by the
careful reviewer.
The coding style should also say that function parameter shall not
be changed in the function. Create your own local copy if you need
it.
I know where this comes from, but this surely cannot be a rule anymore
since this defeats any benefit from move operators.
AFAIU move operator has zero benefits on POD, copy will happen; it only
works on classes or containers that support it (eg vector). But to be
honest I prefer that complex types are always using const ref or rvalue
(|&&) |if you want to have move semantics.
I prefer the style
where a function that only works on a copy receives the argument by
value.
I prefer to choose explicitly whether to use copy or const ref :-)
Not only this is simpler to understand as a principle and in an
interface, but it leaves the compiler (and when necessary, the
programmer) the possibility of moving the argument instead of copying it.
And it can kill the performance if you pass a big variable that is not
movable, AFAIU (I may be wrong).
But I agree that new move functionality in C++11 is really nice and I
know that I have to unlearn the good practice I learned so many years
ago in order to use them efficiently.
Thanks,
Abdel.
