Well, the second document does indeed give some darn good arguments for
trailing braces: 1. "
as shown to us by the prophets Kernighan and Ritchie
"; 2. "
K&R are _right_ and [...] K&R are right
"; 3. "
think 25-line terminal screens here
"
The document also has other great suggestions that we may want to adopt."
Tabs are 8 characters, and thus indentations are also 8 characters
"
The fact is: with trailing braces and steadfast refusal to use vertical
space to improve
code readibility, you do need a tab width of 8 or so to be able to
recognize code blocks.
Incidentally, there's actually one recommendation in that document that IS
worth
adopting *(and in which pretty much all style guides agree plus-minus 5
characters or so):*
The limit on the length of lines is 80 columns and this is a
strongly preferred limit.
Let me repeat that:
The limit on the length of lines is 80 columns and this is a
strongly preferred limit.
And again:
The limit on the length of lines is 80 columns and this is a
strongly preferred limit.
That is the only thing worth really considering from that document.
We can safely consider the 25-line limit a thing of the past unless you
program on
your mobile phone. However, there is a good justification for the 80-column
limit: merging code, where it's useful to see the code side by side. When
resolving merge conflicts in git, we are usually dealing with 3 versions
that need
to be compared. Those three columns won't fit on my screen when lines are
120+
characters long.
I digress. Back to the braces issue. Unlike the arguments for trailing
braces
(K&R did it in a printed book to save paper; I'm a retro programmer who
likes to
do things old school; it obscures my code so people will think I'm really
awesome
because they can't really understand what I'm doing without lots of effort,
just like
Derrida and Piaget didn't write to be understood --- they wrote NOT to be
understood),
so unlike these arguments, there are very good arguments for having opening
braces
on a new line and vertically aligned with the closing bracket.
1. It's much easier to recognize logical code blocks. It is. Try it out.
Read code that you
haven't written and be honest with yourself. It may seem unfamiliar at
first, but once
you are used to it, there's no turning back.
2. If you want to temporarily disable a condition (e.g. in debugging), you
just comment
out the if statement. You do not have to THEN put an opening brace at
the beginning
of the line (extra editing) and remove it later (extra editing again)
--- which also
FORCES you to be inconsistent. (You can't always remove the braces
because
of variable scoping). You have the same issue when you want to use
preprocessor
commands to have conditions only under certain circumstances, so instead
of
#ifdef ENABLE_CONDITION
if (condition)
#endif
{
you'd have to write
#ifdef ENABLE_CONDITION
if (condition) {
#else
{
#endif
3. Which of the two makes it easier to see where the condition actually
ends?
if (StaticData::Instance().IsDetailedTranslationReportingEnabled() ||
StaticData::Instance().IsDetailedALLTranslationReportingEnabled()) {
StaticData::Instance().....
or
if (StaticData::Instance().IsDetailedTranslationReportingEnabled() ||
StaticData::Instance().IsDetailedALLTranslationReportingEnabled())
{
StaticData::Instance().....
In my opinion, the advantages of leading over trailing braces are very
clear. And there are first
converts. Ask Matthias.
There's also an brief but illuminating explanation of the historical
reasons for trailing braces
here:
http://programmers.stackexchange.com/questions/236995/why-do-c-developers-newline-opening-brackets
,
which also hints at readability studies that appear to support my point.
Finally, let's look at the Apple "goto fail" security bug. What happened
there?
This is a snippet of the code in question:
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
goto fail;
goto fail;
The second goto fail was copied in error, it was always executed and lead
to skipping an
important security check. Apart from the fact that 'goto' is almost always
a bad idea (but
we could run into the same problem with switch statements and conditional
break;),
some people argue that this just proves that you should always, always,
always, always
use braces around blocks
So instead of, say
if(condition a)
return 1;
if(condition b)
return 2;
do_more_stuff_here;
we'd have to write
if(condition a) {
return 1;
}
if(condition b) {
return 2;
}
what was that again about saving vertical space? With trailing braces, code
like the goto fail bug
code looks perfectly legitimate. With opening braces on a separate line, it
is immediately clear that
there's something amiss when two consecutive lines after an if statement
or a for are at the same
indentation level. Either only the first line should be indented (no braces
used at all), or there
should be an opening brace between the if and the first line of the
indented block. It may not be
clear what the programmer's intention was, but at least we can see that
something is wrong here.
Trailing braces are bad, tedious and dangerous coding style. Period.
- Uli
On Tue, May 26, 2015 at 6:39 PM, Hieu Hoang <[email protected]> wrote:
> ah, the style guide that no-one has ever bothered to follow since I
> originally wrote it 10 years ago.
>
> I changed it in Jan to reflect the style that people actually check in, so
> presumably prefer. The beautifier tries to mimick this style. I will change
> the style guide back.
>
> fyi - the original style is based on microsoft, eg
> http://blogs.msdn.com/b/brada/archive/2005/01/26/361363.aspx
> i guess people prefer a unix-based style, eg
> https://www.kernel.org/doc/Documentation/CodingStyle
>
>
> I think beautifying is a necessary 'cos we have a distributed project of
> committers who use different editors and OSes. It provides some consistency.
>
> I regularly see you commit changes where you've manually wrap long lines.
> Youre welcome to change the beautifier to enforce this. However, I prefer
> that the beautifier isn't changed too much otherwise the code will flip
> flop for no good reason.
>
>
>
> Hieu Hoang
> Researcher
> New York University, Abu Dhabi
> http://www.hoang.co.uk/hieu
>
> On 26 May 2015 at 20:30, Ulrich Germann <[email protected]> wrote:
>
>> Hi Jeroen,
>>
>> on the matter of style I'd like to point out that the official style
>> guidelines for Moses code require opening braces on a separate line.
>>
>> http://www.statmt.org/moses/?n=Moses.CodeStyle
>>
>> The official style has always required this since the first entry about
>> this in the Wiki back in 2009. Recently (Jan 16 this year) the page was
>> vandalized by an anonymous editor to claim something to the contrary, but
>> the page has now been restored to what it should be. So while you're
>> fiddling with the beautify scripts, please fix this as well.
>>
>> I'm personally no friend of automatic "beautification", because it does
>> far more harm than good in my opinion, but if you insist on doing it, it
>> should be done right.
>>
>> Best regards - Uli
>>
>> On Sun, May 17, 2015 at 2:13 PM, Jeroen Vermeulen <
>> [email protected]> wrote:
>>
>>> Hi all,
>>>
>>> We have a replacement for the old beautify.perl script:
>>> scripts/other/beautify.py.
>>>
>>> It does one of two things, or both:
>>> * Re-format C/C++ source code, just like the old script did.
>>> * Check for style errors and such.
>>>
>>> This last thing is called a "lint" check. For this I chose Pocketlint,
>>> a checker I have good experiences with, although if people want a
>>> different one (or additional checks) I can change that.
>>>
>>> I fixed most of the lint that got reported, except in JavaScript code.
>>> We may add automatic reformatting for additional languages later. I
>>> sincerely hope all of this does not cause any serious merge problems for
>>> your branches.
>>>
>>> Ideally, everyone would get in the habit of installing Pocketlint and
>>> running this script regularly whether they accidentally added any lint.
>>> To see how the script works, run:
>>>
>>> ./scripts/other/beautify.py -h
>>>
>>> The lint check processes a few files at a time. By default it stops
>>> when it sees lint. If you want to see a full check, use the -i option.
>>>
>>>
>>> Jeroen
>>> _______________________________________________
>>> Moses-support mailing list
>>> [email protected]
>>> http://mailman.mit.edu/mailman/listinfo/moses-support
>>>
>>>
>>
>>
>> --
>> Ulrich Germann
>> Senior Researcher
>> School of Informatics
>> University of Edinburgh
>>
>> _______________________________________________
>> Moses-support mailing list
>> [email protected]
>> http://mailman.mit.edu/mailman/listinfo/moses-support
>>
>>
>
--
Ulrich Germann
Senior Researcher
School of Informatics
University of Edinburgh
_______________________________________________
Moses-support mailing list
[email protected]
http://mailman.mit.edu/mailman/listinfo/moses-support
