[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [cdt-dev] Code Style for CDT code
|
> We can discuss this on code review, but if we disagree, like in this case, we need to
> discuss this with all committees. Especially if deal is "No I would not remove -1 unless
> you fix the braces".
Agreed. But to be fair, I didn't say that. I asked what was your reason to
"strongly disagree" with putting braces. I also mentioned that we had a problem with
a missing brace before, which is why I was recommending adding them.
It is not ok to say "No I would not remove -1 unless you fix the braces".
But I am comfortable with saying "No I will not remove my -1 unless you answer the
question of the reviewer".
Funny thing is, after talking with Marc Dumais, it turns out I was wrong about
asking for braces in this particular review because it was an if-else :) For
example:
if (a)
System.out.println();
else {
a = 8;
return;
}
There is no risk of adding an extra line after the sysout because the compiler
will know right away it is a mistake (orphaned else statement). I hadn't
realized that. But you also didn't tell me :)
> I still don't understand where all these rules are coming from, if there "developers
> guide for dsf" document somewhere which I am missing?
They are not really rules. They are just recommendations based on experience.
Sometimes they're valuable recommendations, sometimes less so. But that
is what reviews are for, in part.
[...]
> So every extra bracket you put in which takes separate line reduces amount
> of real code that can fit in the screen which significantly reduce ability of
> another developer to understand the code.
As reviewers, we have to judge what is best in each case. This above justification
is valid in some cases and I hadn't thought of it. I'll include it in my reviewing 'arsenal' :)
But in some cases (like the bug in the iPhone), it is not worth the risk.
Marc
