trinity-devel@lists.pearsoncomputing.net

Message: previous - next
Month: January 2015

Re: [trinity-devel] Codebase formatting discussion

From: Michele Calgaro <michele.calgaro@...>
Date: Fri, 09 Jan 2015 12:01:58 +0900
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 01/09/2015 11:40 AM, Timothy Pearson wrote:
> <snip>

> 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();
> 

I think what is desired is something like this:

do_something();
switch(foo)  {
    case bar:
        a=1;
        break;
    case baz: {
        a=2;
        ...long case block...
        c=4;
        break;
    }
    case asd:
        a=3;
        break;
    default:
        a=0;
}
do_something_else();

The { and } in case blocks are not really needed and my preference would be not to use them there. If needed, they
should be indented as any other block. The style in GIT adds an extra indentation level to those {-} case blocks,
which is what Slavek and I were "complaining".

> <snip> 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?

My preference would be for breaking after the logical operator and either have no indentation or double indentation
for the subsequent lines.

if ((disktype & TDEDiskDeviceType::CDROM) ||
   (disktype & TDEDiskDeviceType::CDR) ||
   (disktype & TDEDiskDeviceType::CDRW))

or

if ((disktype & TDEDiskDeviceType::CDROM) ||
       (disktype & TDEDiskDeviceType::CDR) ||
       (disktype & TDEDiskDeviceType::CDRW))

> 
> 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?

Both "a=1" and "a = 1" are fine for me. For long time I used the first way, but in the last couple of years I switched
ti the second one: usually more readable for simple expressions but can become confusing with long and complex ones
such as:
if ((a = 2) && (++b != (c + (x * y) / f(q,w,e))))
Choice is up to you and Slavek.

> 
> 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.
> 
Same as you, no spaces at the beginning and end parenthesis, i.e. line 2518 is better for me.

Also, I suggest to have a space between function parameters as in
foo(par1, par2, par3, par4)
instead of
foo(par1,par2,par3,par4)

Cheers
  Michele
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJUr0SlAAoJECp1t8qK3tXP7qUP/1RiMeDP7yL5eSRZE2jR/lDV
3rOwYMr2YWJt7fiC7IG61YP0l1FnASol3ZSeqv2NA6jeyjI8ggw4MEg24RfM9xGo
weQQV8e/yXwH1t2l03zydgJumkDRXNXG3kGmFA+dUrZBck0cqbJ6pyEj//PD/+Zk
1fWgHOJrVU76jknoncegf4LuZ9q4VmiwXBZqEfR/5f+EtLXG4lwec/v0Hf/qRGvc
TcmmFEt2Qu7R4E7YrBAlo1vPfD7NtEx0a3tl5YcyoAOsS0iRds9lPOGSrK1s4878
3RjE1GcUyhIth+FlB7bzP3Y09sylyIhWwJtF8cBqq7ig1YYYy56BMDlnMWnRT0e0
L0SkTqaBp3bhXBfpBl/DVJ6XLy5NC1CaNdaMUW4dbUGwObSS5pit+slOKCK6UwWw
UCR5aUUfm1SfMV3QuJqwIJ1HDkovvWrDVGoxCiS3VORwkPMhen7eZbvqGoJpVJx9
uZ+MhtgYn1skENQXniAKYozU4hX7GYDNDxcWerZ7TFYTjWhTsICeOTOE8ibuFc5L
UqKF7+lwnSWtIPJD2kQk72CZyWjr9aaKSvLH2cQt0BAWKqWJyXCzWaKUiRIVkD6p
76raXrv/tUhQ8jw7s1zGnSAaS5ZUzIEaqeaO+9AyPuOqjSLn/hbzH0P//g49zAKS
mdwk39AvBf4ZQUsPuq1U
=f7Ag
-----END PGP SIGNATURE-----