Complete.Org: Mailing Lists: Archives: freeciv-dev: December 2001:
[Freeciv-Dev] Re: Voting results for first part of style guide questionn
Home

[Freeciv-Dev] Re: Voting results for first part of style guide questionn

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: rf13@xxxxxxxxxxxxxxxxxxxxxx
Cc: freeciv development list <freeciv-dev@xxxxxxxxxxx>
Subject: [Freeciv-Dev] Re: Voting results for first part of style guide questionnaire
From: Petr Baudis <pasky@xxxxxxxxxxx>
Date: Mon, 3 Dec 2001 22:03:18 +0100

Dear diary, on Mon, Dec 03, 2001 at 05:44:43PM CET, I got a letter,
where Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> told me, that...
> So these are the result:
> 
>  * 1: init vars 
>  * 2: empty line after vars 
>  * 3: comments code above and not right and not below
>  * 4: extra {} on iterates 
>  * 5,6: always put braces on if and else blocks
>  * 7: merge declaration lines
This shouldn't be asserted too agressively imho. It's not even technically
possible, when you want long comment for each of those variables :-).

>  * 8: allow double dereferencing in declaration lines
>  * 9: comments in declarations are at the right and not above
I would also call this rather 'preferred', as this can't be absolute, if
you have two pages comment explaining that, it's generally against common
sense :-). This wouldn't be so fatal anyway..

Basically what I already said - don't force those rules blindly, especially
when they concradict common sense and don't trash patches just for few
_little_ violations.

> So if there is anybody who can't live with this: speak up now.
Amen.

> And now to the second part. Gregory Berkolaiko has collected these
> questions:
> 
> /*******************
>  * 10: long comments
>  *******************/
> int foo10(int x)
> {
> 
>   /* A */
>   /* blah
>      blah blah
>      blah */
>   if (x==0) 
>     return 1;
> 
>   /* B */
>   /* blah
>    * blah blah
>    * blah */
>   if (x==1)
>     return 0;
> 
>   /* C */
>   /* 
>    * blah
>    * blah blah
>    * blah 
>    */
>   return 42;
> }
B, C for very long ones.

> 
> /**********************
>  * 11: braces in switch
>  **********************/
> int foo11(int x)
> {
> 
>   /* A (always braces) */
>   switch(x) {
>   case 2:
>     {
>       return 2;
>     }
>   case 3:
>     {
>       int d = 5;
>       return d-x;
>     }
>   }
> 
>   /* B (braces where needed) */
>   switch(x+3) {
>   case 2:
>     return 2;
>   case 3:
>     {
>       int d = 5;
>       return d-x;
>     }
>   }
> 
>   /* C (no extra local variables) */
> 
>   {
>     int d;
>     switch(x) {
>     case 2:
>       return 2;
>     case 3:
>       d = 5;
>       retrun d-x;
>     }
>   }
> }
B! _BUT_:
  switch(x+3) {
    case 2:
      return 2;
    case 3:
      {
        int d = 5;
        return d-x;
      }
  }

Having case indented is much cleaner, more readable and more systematic (as if 
we indent
contents of blocks, we should indent all of them).

> 
> /********************************
>  * 12: empty lines between blocks 
>  ********************************/
> int foo(void)
> {
> 
>   /* A */
>   while (*i++) {
>     ++j;
>   }
> 
>   if (j > 10) {
>     foo();
>   }
> 
>   /* B */
>   while (*i++) {
>     ++j;
>   }
>   if (j > 10) {
>     foo();
>   }
> }
A!

> 
> /******************
>  * 13: empty blocks 
>  ******************/
> int foo13(void)
> {
> 
>   /* A */
>   while (*i++) {
>     continue;
>   }
> 
>   /* B */
>   while (*i++) {
>   }
> 
>   /* C */
>   while (*i++) {}
> 
>   /* D */
>   while (*i++);
> 
>   /* E */
>   while(*i++) {
>     /* nothing */
>   }
> 
>   /* F */
>   /* !disallow such constructs! */
> }
D, but i think it doesn't really matter, as those cases are pretty rare.

> 
> /******************************
>  * 14: Comments in conditionals
>  ******************************/
> int foo14(void)
> {
>   int x=0;
>   
>   /* A */
>   if (is_barbarian(pplayer)) {
>     x++
>   /* If not barbarian, ... */
>   } else {
>     x--;
>   }
> 
>   /* B */
>   if (is_barbarian(pplayer)) {
>     x++;
>   } else { /* If not barbarian, ... */
>     x--;
>   }
> 
>   /* C */
>   if (is_barbarian(pplayer)) {
>     x++;
>   } else {
>     /* If not barbarian, ... */
>     x--;
>   }
> 
>   /* D */
>   if (is_barbarian(pplayer)) {
>     x++;
>   } else { /* if (is_barbarian(pplayer)) */
>     x--;
>   }
> }
A, or C, or maybe even B, altough not ideal. I stronly disagree D, as this
has anyway mean only for long conditionals and long blocks, so this will
be overkill then.

-- 

                                Petr "Pasky" Baudis

UN*X programmer, UN*X administrator, hobbies = IPv6, IRC, FreeCiv hacking
.
  "A common mistake that people make, when trying to design
   something completely foolproof is to underestimate the
   ingenuity of complete fools."
     -- Douglas Adams in Mostly Harmless
.
Public PGP key, geekcode and stuff: http://pasky.ji.cz/~pasky/


[Prev in Thread] Current Thread [Next in Thread]