trinity-devel@lists.pearsoncomputing.net

Message: previous - next
Month: January 2015

Re: [trinity-devel] Codebase formatting discussion

From: "Timothy Pearson" <kb9vqf@...>
Date: Thu, 8 Jan 2015 20:40:45 -0600
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA224

<snip>
> Funny enough, I also had to code for several years with twin indentation
> style.
> One thing that I can say for sure about styles is that no matter what
> style you use, if you use it long enough you get
> used to it and even what at first might have seem ugly, in the end becomes
> natural. twin style is a perfect example of
> that: at first I really disliked it, but after I got used to it, it was no
> longer a problem. Not my favorite style though.

My personal study of these adaptation phenomena has led me to the
conclusion that, in most cases, the "adaption" is more an automated hiding
of an inefficient and/or error-prone style from the level of the
programmer's conciousness.  Basically it shows up to an outside observer
as a slowing down of the programmer's work without his or her knowledge,
similar to clunky computer interfaces slowing down common tasks without
the user's knowledge or inefficient factory stations draining employee
productivity.  It can even manifest as a subtle almost subconcious dislike
of working on the code without the programmer ever really becoming aware
of why.  This is something we want to avoid if at all possible, hence this
discussion. :-)

Now of course there are cases where true adaption does take place, but
there are limits on how far out the style can be from the programmer's
natural expectations before this no longer holds. ;-)

>
>> I am aware that the more popular now are brackets at the end of lines.
>> And I, as well as Michele, earlier mentioned
>> that we are flexibile and we have no problem adapt to it. In fact, it
>> was enough for the pleasure to me knowing
>> that along with Michele we have the same preference :)
>
> I am flexible, so up to the BDFL's decision.
>
>> But what would prefer a smaller indentation in the switch cases - as
>> featured Michele. Unfortunately I do not know
>> whether astyle can be set to such a manner. See attachment.
>
> Yes, I think the indentation for case blocks that I proposed is more
> logical: the case blocks are indented as a normal
> block. Instead the style in GIT adds an extra indentation level, which is
> somehow counter-intuitive. Just for the sake
> of consistent indentation.

So, to be clear, this is what is desired?

do_something();
switch(foo)  {
case bar:
    a=1;
case baz:
    a=2;
case asd:
    a=3;
default:
    a=0;
}
do_something_else();

My question is this:  Say I have a long case statement (dozens of cases
with subsequent processing blocks, enough to run off both vertical edges
of my screen) and it's "conveniently" located in the middle of a large
source file with other case blocks and "normal code" above and below it. 
With the style above I will have problems finding the end of the case
block, which is why I started indenting them like this:

do_something();
switch(foo)  {
    case bar:
        a=1;
    case baz:
        a=2;
    case asd:
        a=3;
    default:
        a=0;
}
do_something_else();

There are actually several instances of the pathological case described
above in the TDE source tree right now, in the udev and network-manager
interfaces among other locations, and I expect the number of these to grow
as more DBUS-based system abstraction layers are forced on us (e.g.
systemd, logind, and who knows what else down the line).  Being able to
easily say "yeah, I know what that case block does, get me to the next
line of code after the case handler" by simply following the indentation
proved quite valuable for me when writing the TDE hardware library.

Rebuttals welcome. :-)

> That was only a suggestion. I also thing it is awkward to work like that
> and just easier to adapt to a new style.
> I did some tests and as long as most options are the same in the two
> styles, it seems to be quite ok.
> Also IMO we have to use point d) before any commit to make sure the
> committed code follows the correct style
> (especially when adapting from another style, it is easy to inadvertently
> introduce some style mistakes).
> Alternatively, we can have an automated beautify task running regularly on
> GIT (for example once a week).

I will definitely enforce the chosen style through an automated method;
I'm not sure if I can practically hook into GIT for pre-commit as we have
so many modules but I will be adding something to the server to make sure
the style doesn't inadvertently "drift" over time.

This is the current consensus as far as I can tell:
1.) Every conditional uses braces
2.) Hard tabs
3.) Conditional indentation like so:

if (foo) {
    a=0;
}
else if (bar) {
    a=1;
}
else {
    a=2;
}

We still need to discuss how to handle large chains of conditionals, e.g.
these pathological cases:
https://git.trinitydesktop.org/cgit/tdelibs/tree/tdecore/tdehw/tdehardwaredevices.cpp?id=3d06098ece39d7a2e2d292b28fd0e165b3b4f43e#n2475

Is the style shown there acceptable?

Also on the docket are how to handle simple variable assignments; there
are several ways I've seen:
a=2;
a = 2;
a= 2;
a =2;

I'm fairly undecided on this.  For consistency with larger statements "a =
1;" should probably be used but it seems such a waste of space compared
with "a=1;".  Thoughts?

One other issue is the whitespace within conditional expressions.  As you
can see I'm fairly undecided on this as well, having used multiple styles
already:
https://git.trinitydesktop.org/cgit/tdelibs/tree/tdecore/tdehw/tdehardwaredevices.cpp?id=3d06098ece39d7a2e2d292b28fd0e165b3b4f43e#n2517
https://git.trinitydesktop.org/cgit/tdelibs/tree/tdecore/tdehw/tdehardwaredevices.cpp?id=3d06098ece39d7a2e2d292b28fd0e165b3b4f43e#n2518

Given a choice I slightly prefer the style shown on line 2518; my only
strong preference here is that I don't want to see any whitespace between
two matched parenthesis:
Correct: execute_something()
Wrong:   execute_something( )

Thoughts are welcome here as well.

Thanks!

Tim
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iFYEARELAAYFAlSvP5kACgkQLaxZSoRZrGHZmADeKfE2P9f9cNJH/4pI41324VsL
zoL3H5ZX/jj99gDfZdDMekzA7LYtee4HN+kg4HSGqIHk6PbNaMELhg==
=Lb0y
-----END PGP SIGNATURE-----