[Freeciv-Dev] Re: Coding Guideline Updated RFC

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Nov 26, 2001 at 12:29:41PM -0500, Andrew Sutton wrote:
> On Monday 26 November 2001 10:51 am, you wrote:
> 
> 1B 2B 3B 4A 5A 6A 7(see comment) 8A
> 
> > > /***************
> > >  * 1: init vars
> > >  ***************/
> >
> > 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
> 
> i thought that C - at least in its pure form didn't allow you to initialize 
> values on the stack like that: they have to be declared first - reason being: 
> it's not C++ there's no such thing as constructors in C.

It looks like you have done to less C before you started C++ ;) No the
language allows it.

> for C, i really like seeing all the variables declared and
> commented:
> 
> int some_int;         /* used for... */
> float some_float;     /* used for ... */

Currently you are the only one with this opinion.

> > > /****************
> > >  * 3: comments
> > >  ****************/
> >
> > B does not depend on the line's length
> 
> right.. comment before code. except in structural declarations where the 
> comment should be on the same line for easier readibility.
> 
> struct foo
> {
>       int bar;        /* blah blah blah */
> };

Yes and no. It is ok if the space is sufficient like here:
  F_PARTIAL_INVIS,    /* Invisibile except when adjacent (Submarine) */
  F_SETTLERS,         /* Does not include ability to found cities */
  F_DIPLOMAT,
  F_TRIREME,          /* Trireme sinking effect */
  F_NUCLEAR,          /* Nuclear attack effect */
  F_SPY,              /* Enhanced spy abilities */
  F_TRANSFORM,        /* Can transform terrain types (Engineers) */

but fails otherwise:
  AB_NOT_BUILD_LOC,             /* City is not allowed to be built at
                                   this location */
  AB_NOT_ADDABLE_UNIT,          /* Unit is not one that can be added
                                   to cities */
  AB_NOT_BUILD_UNIT,            /* Unit is not one that can build
                                   cities */
  AB_NO_MOVES_BUILD,            /* Unit does not have moves left to
                                   build a city */
  AB_NO_MOVES_ADD,              /* Unit does not have moves left to
                                   add to city */
  AB_NOT_OWNER,                 /* Owner of unit is not owner of
                                   city */
  AB_TOO_BIG,                   /* City is too big to be added to */
  AB_NO_AQUEDUCT,               /* Adding takes city past limit for
                                   aquaduct but city has no
                                   aquaduct */
  AB_NO_SEWER                   /* Adding takes city past limit for
                                   sewer but city has no sewer */

Always above:

  /* City is not allowed to be built at this location */
  AB_NOT_BUILD_LOC,

  /* Unit is not one that can be added to cities */
  AB_NOT_ADDABLE_UNIT,

  /* Unit is not one that can build cities */
  AB_NOT_BUILD_UNIT,

  /* Unit does not have moves left to build a city */
  AB_NO_MOVES_BUILD,

  /* Unit does not have moves left to add to city */
  AB_NO_MOVES_ADD,

  /* Owner of unit is not owner of city */
  AB_NOT_OWNER,

  /* City is too big to be added to */
  AB_TOO_BIG,

  /* Adding takes city past limit for aquaduct but city has no
     aquaduct */
  AB_NO_AQUEDUCT,

  /* Adding takes city past limit for sewer but city has no sewer */
  AB_NO_SEWER

You may want to remove the extra blank lines

> > > /****************************
> > >  * 7: merge declaration lines
> > >  ****************************/
> >
> > declarations should by grouped by their meaning if possible
> > besides what if there are too many ints to fit on one line?
> 
> actually, you should separate out all declarations so they have their own 
> line. i know its redundant and people don't like it but it will make complex 
> functions oh so much more readable. especially if they declare alot of 
> variables. then, you get the option of commenting them as they're declared:
> 
> int foo()
> {
>       int a;          /* used for ... */
>       int b;          /* used for ... */
>       int c;          /* used for ... */
> 
>       // code...
> }

As I said you are alone here.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Like the ad says, at 300 dpi you can tell she's wearing a
  swimsuit. At 600 dpi you can tell it's wet. At 1200 dpi you
  can tell it's painted on. I suppose at 2400 dpi you can tell
  if the paint is giving her a rash."
    -- Joshua R. Poulson