[Freeciv-Dev] Re: Coding Guideline Updated RFC

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

My own voting: 
1A! 2B  3B  4A  5-  6A  7A  8A

Comments below

 --- Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx> wrote: 
> Following suggestion of Raimar below is enlarged questionnaire.
> 
> ============ 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;
> }

when variable is initialised immediately it's so much clearer what it's
meaning is.  especially if it's not going to change throughout the
function's body

A!

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

B does not depend on the line's length

> /**************************
>  * 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 by all means
look what indent -kr -i does to it otherwise!


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

A is more orderly but takes more space...

> /**************************************************** 
>  * 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 is better


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

declarations should by grouped by their meaning if possible
besides what if there are too many ints to fit on one line?
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);
> }

A
why not

__________________________________________________
Do You Yahoo!?
Everything you'll ever need on one web page from News and Sport to Email and 
Music Charts
http://uk.my.yahoo.com