[Freeciv-Dev] Re: Please vote!

[Reinier Post <rp@xxxxxxxxxx>]

On Wed, Nov 28, 2001 at 12:37:05PM +0000, Gregory Berkolaiko wrote:
> Please vote more actively on the Coding Guideline Updated RFC
> 
> Only 8 people have voted so far:
> 
> Greg Wooledge:
> Daniel Sjölie
> Raimar Falke
> Tony Stuckey
> Petr Baudis
> Andrew Sutton
> Mike Kaufman 
> and meself
> 
> (If you've sent an email but I haven't recorded you, please shout!)

Only a few lines of the code are mine, so for what it's worth ..

>> ============ Cut here.  Fill in blue ink only ==================
>> 
>> /*************** 
>>  * 1: init vars 
>>  ***************/
>> 
>> /* A */
>> int foo1a(struct city *punit)
>> {
>>   int x = punit->x;
>> }
>> 
>> /* B */
>> int foo1b(struct city *punit)
>> {
>>   int x;
>> 
>>   x = punit->x;
>> }

B.  There's a difference in meaning for 'static' variables:

| #include <stdio.h>
| 
| static int x()
| {
|   static int the_x = 1;
|   return ++the_x;
| }
| 
| int main(int argc, char *argv[])
| {
|   printf("%d\n", x());
|   printf("%d\n", x());
|   printf("%d\n", x());
| }

prints

2
3
4

(It doesn't without the 'static'.)
It seems clearer to restrict the syntax to static var initializations only.

>> /**************************
>>  * 2: empty line after vars 
>>  **************************/
>> 
>> /* A */
>> int foo2a(void)
>> {
>>   int x;
>>   x = 3;
>> }
>> 
>> /* B */
>> int foo2b(void)
>> {
>>   int x;
>> 
>>   x = 3;
>> }

B.

>> 
>> /****************
>>  * 3: comments 
>>  ****************/
>> 
>> /* A */
>> int foo3a(int x)
>> {
>>   x = 3;                     /* assign 3 to x */
>> }
>> 
>> /* B */
>> int foo3b(int x)
>> {
>>   /* assign 3 to x */
>>   x = 3;
>> }

No preference - I use both.  I also use

   /* B */
   int foo3b(int x)
   {
     x = 3;
       /* assign 3 to x */
   }

>> /**************************
>>  * 4: extra {} on iterates 
>>  **************************/
>> int foo4(struct city *pcity)
>> {
>> 
>>   /* A */
>>   unit_list_iterate(pcity->units_supported, punit) {
>>     kill(punit);
>>   } unit_list_iterate_end;
>> 
>>   /* B */
>>   unit_list_iterate(pcity->units_supported, punit)
>>       kill(punit);
>>   unit_list_iterate_end;
>> }

A.  Extra {}s are always good.

>> /****************************************** 
>>  * 5: unnecessary braces after conditionals 
>>  ******************************************/
>> int foo5(int x)
>> {
>> 
>>   /* A */
>>   if (x == 3) {
>>     return;
>>   }
>> 
>>   /* B */
>>   if (x == 4)
>>     return;
>> }

A.  (This is the first case where I really care.)

>> 
>> /**************************************************** 
>>  * 6: unnecessary braces after conditionals with else 
>>  ****************************************************/
>> int foo6(int x)
>> {
>> 
>>   /* A */
>>   if (x == 3) {
>>     return 1;
>>   } else {
>>     return 0;
>>   }
>> 
>>   /* B */
>>   if (x == 4)
>>     return 1;
>>   else
>>     return 0;
>> }

A.  (id)

>> /****************************
>>  * 7: merge declaration lines
>>  ****************************/
>> 
>> /* A */
>> int foo7a(struct city *pcity)
>> {
>>   int total, cost;
>>   int build = pcity->shield_stock;
>> }
>> 
>> /* B */
>> int foo7b(struct city *pcity)
>> {
>>   int total, cost, build = pcity->shield_stock;
>> }

A slight preference for A.  I wasn't sure if in

| #include <stdio.h>
| 
| int main(int argc, char *argv[])
| {
|   char c,d = 65;
|   printf("%c\n", d);
| }

d would be an int.  (It isn't; the program prints 'A'.)

>> /*************************
>>  * 8: double dereferencing
>>  *************************/
>> 
>> /* A */
>> int foo8a(struct city *pcity)
>> {
>>   struct player *owner = city_owner(pcity);
>>   struct nation_type *nation = get_nation_by_plr(owner);
>> }
>> 
>> /* B */
>> int foo8b(struct city *pcity)
>> {
>>   struct player *owner = city_owner(pcity);
>>   struct nation_type *nation;
>> 
>>   nation = get_nation_by_plr(owner);
>> }

How is this different from case 1?  Or do you mean

   /* B */
   int foo8b(struct city *pcity)
   {
     struct nation_type *nation;
  
     nation = get_nation_by_plr(city_owner(pcity));
   }

?

I think all are fine.

More cases:

  /*********************************/
   * 9: empty lines between blocks *
  /*********************************/

  /* A */

  while (*i++)
  {
    ++j;
  }

  if (j > 10) {
    foo();
  }

  /* B */

  while (*i++)
  {
    ++j;
  }
  if (j > 10) {
    foo();
  }

(I prefer A. Has there been a vote on all the indent(1) options?)

  /*******************/
   * A: empty blocks *
  /*******************/

  /* A */

  while (*i++)
  {
    continue;
  }

  /* B */

  while (*i++)
  {
  }

  /* C */

  while (*i++) {}

  /* D */

  while (*i++);

(I prefer C.)

There is probably more (whitespace after commas in function calls?).

-- 
Reinier