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

[vze2zq63@xxxxxxxxxxx]

Raimar Falke wrote:

> Gregory Berkolaiko has put together the results of the first part of
> the style guide questionnaire:

10 B or C
11B?
12A
13E
14C

> 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;
> }

10A can be reformatted automatically by indent.  In other personal 
projects I find this to be a good thing, but in FreeCiv this means 
changing one part of the comment might have indent reformat the whole 
thing, making CVS annotate on it work incorrectly.
Either of the latter two are acceptable to me.


> /**********************
>  * 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;
>     }
>   }
> }

by 11A, do you mean to *always* but braces into *every* switch, or just 
that a switch must have all braces or no braces?  I think the former 
would be very bad, but I assume you mean the latter.
C is bad, since it can lead to obfuscation if many variables are used. 
Between the other two there's not much difference - I'll go with B since 
it's the smallest change from what we have now.

> /******************
>  * 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! */
> }

This seems to be getting a bit arbitrary.  13E looks the most legible - 
but such constructs should IMO be avoided except where they're truly 
needed.  A bigger question might be between:
for(i=0, j=1; i<=10; i++, j *= 2) {
  /* nothing */
}

and

j=1;
for (i=0; i<=10; i++) {
  j *= 2;
}

I prefer the latter one, but many times it's not this cut-and-dry.


> /******************************
>  * 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--;
>   }
> }

I really dislike A, and somewhat dislike D.  Either B or C is fine.

Really this comes back to the earlier survey:  "3.  Comments code above 
and not right and not below" should rule out A, B, and D.  I guess that 
leaves C.

jason