Re: [cdt-dev] Code Style for CDT code

[Alena Laskavaia <elaskavaia.cdt@xxxxxxxxx>]

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".

I still don't understand where all these rules are coming from, if there "developers guide for dsf" document somewhere which I am missing?

When people do all these "safe coding" recommendation they don't consider impact of decision.
For example in Canada it is mandatory to wear bicycle helmet because of safety, and it clearly may some lives and nobody can argue that,
however some research shows that this negatively impacts people desire to use bike and as a result have general negative impact on overall population health.

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.

On Tue, Jan 27, 2015 at 1:18 PM, Marc Khouzam <marc.khouzam@xxxxxxxxxxxx> wrote:

> Marc yet again -1'ed my commit because of code style, specifically because
> not surrounding a single statement with braces within if operator.

We have to differentiate between "code-style" and "safe coding practices".
For example putting braces on the same line or on a separate line.  To me,
that is "code-style".  I've never put a -1 on a review because of that and
most probably never mentioned it in a comment (I like the same-line-brace).

Using braces all the time goes beyond style for me; it is a recommendation
for a "safer coding practice".  If a reviewer feels we can decrease the risk of future
bugs with a small change in the code, I think it should be mentioned in the review.
In such cases, I do my best to explain why I suggest the change (although
maybe not in great detail).

Googling around, one can find multiple references recommending to always
use braces.  Interestingly, I just found out that using braces would have
avoided the recent "iPhone critical security bug":
http://www.wired.com/2014/02/gotofail/

I personally think that an open discussion in the review/bugzilla should be
sufficient to reach an agreement on style vs coding practice.

Marc

---

From: cdt-dev-bounces@xxxxxxxxxxx [cdt-dev-bounces@xxxxxxxxxxx] on behalf of Alena Laskavaia [elaskavaia.cdt@xxxxxxxxx]
Sent: January 27, 2015 8:08 AM
To: CDT General developers list.
Subject: [cdt-dev] Code Style for CDT code

Sorry to bring this up again, but I want to discuss this on cdt-dev.

Marc yet again -1'ed my commit because of code style, specifically because not surrounding

a single statement with braces within if operator. First of all I don't remember agreeing on any sort of code style like that (see cdt process and code style wiki), second of all I strongly disagree with this specific one. Coding style like that was created I think 20 years ago where people were
formatting code manually. Today everybody who works on cdt code should use auto-formatter.

This should be the rule, not braces.
Adding more braces actually clutters the code leading to lesser readability, and it affects all code not just "we had a case 10 years ago when somebody formatted code manually and it led to a bug".

I also strongly disagree with using code style with right side comparison like (null == a). This was created again to mitigate bad gramma of C language and it has no value in Java. It greatly impairs readability because it is not a natural order of english language, so people even they understand it eventually will had hard time reading these expressions.

Would be nice if we can actually agree of code formatting style so we can add this to our save actions, so then we don't have waste time dealing with code style and formatting on the reviews.

Note if majority of committers will agree on certain style, even I personally disagree with it I will follow it.