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

[Marc Khouzam <marc.khouzam@xxxxxxxxxxxx>]

I'm learning a lot today.

Thanks for the counter example John.  I'm glad to see you still hang around these parts :)
________________________________________
From: cdt-dev-bounces@xxxxxxxxxxx [cdt-dev-bounces@xxxxxxxxxxx] on behalf of John Cortell [john.cortell@xxxxxxxxxxxxx]
Sent: January 27, 2015 2:38 PM
To: CDT General developers list.
Subject: Re: [cdt-dev] Code Style for CDT code

Actually, there is a risk. Consider the following addition to the code, which will not do what the author likely intended:

if (a)
  System.out.println();
   if (b)
      System.out.println();
else {
  a = 8;
  return;
}

For this reason, it's good practice to always use braces. See 7.4 in http://www.oracle.com/technetwork/java/codeconventions-150003.pdf

That said, this is not a bible. It contains some recommendations I disagree with and do not follow (hm...kind of like the real bible). So, it's a religious debate, and one I probably shouldn't stick my nose in. But it's been a while and I felt like popping in and saying hi.

John

-----Original Message-----
From: cdt-dev-bounces@xxxxxxxxxxx [mailto:cdt-dev-bounces@xxxxxxxxxxx] On Behalf Of Marc Khouzam
Sent: Tuesday, January 27, 2015 1:16 PM
To: CDT General developers list.
Subject: 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
_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit https://dev.eclipse.org/mailman/listinfo/cdt-dev
_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/cdt-dev