seems like referendums are all the rage at the moment so we're gonna
have 1 to decide coding style.
https://www.surveymonkey.com/s/BN38LBQ
If you work with the Moses code, please vote. This will give us a clear
mandate for future development.
Polls will open until Sunday evening. All results are public
On 27/05/2015 05:16, Jeroen Vermeulen wrote:
On May 27, 2015 2:47:02 AM GMT+07:00, Ulrich Germann
<[email protected]> wrote:
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]
<mailto:[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] <mailto:[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]
<mailto:[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] <mailto:[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] <mailto:[email protected]>
http://mailman.mit.edu/mailman/listinfo/moses-support
--
Ulrich Germann
Senior Researcher
School of Informatics
University of Edinburgh
Personally I'd be in favour of opening braces on separate lines and
80-column lines, but what I did to the beautify script was not meant
to trigger a vast rewrite of the main body of code. Changing that is a
whole separate discussion, involving not just what's best but also how
to deal with change.
>From long experience as a software engineer I can say that automated
code formatting is a good thing, provided it's done regularly and
people adjust their styles to it. In fact I would prefer any style
guide not to talk about things like opening braces at all: "Try to
make your code look like what's already there, and be aware that the
code is automatically reformatted on a regular basis. Run a lint check
before committing."
(Maybe I should make the beautify script accept a list of filenames,
and/or add the option to run only on files with uncommitted changes.)
I'll have a look into making astyle break long lines, but be warned: I
made the lint checker accept lines up to 400 columns only because
there were so many over 300! We may have to ease into it a few lines
at a time.
Jeroen
--
Hieu Hoang
Researcher
New York University, Abu Dhabi
http://www.hoang.co.uk/hieu
_______________________________________________
Moses-support mailing list
[email protected]
http://mailman.mit.edu/mailman/listinfo/moses-support
