[Freeciv-Dev] Re: Government file.

[rutger@xxxxxxxx]

On Thu, Jan 20, 2000 at 10:04:11PM +0000, Nicolas BRUNEL wrote:
> Hello,
> 
>    I had got some free time to modify common/goverment.c .
> 
> It did three things :
> 
>    - I withdraw { } containing one instructions. 
>    - In a loop, if a test is done which can stop the loop, it is an
>      exit condition of the loop. This condition have to be include in the
>      test of exit of the loop( && !this_condition) .
> 
>    Example:
>      for(i = 0; i < size; i++)
>          if (ToBeFound == Tab [i]) return i;
> 
>      becomes
> 
>      for(i = 0; i < size && ToBeFound != Tab [i]; i++);
>      return i;
> 
>    - & Tab[i] is equivalent to Tab+i .
>      IMHO, as it is shorter, Tab+i is better. 

I'm programming for about 8 years in C now, and work for a company for
which programming in C is a part [just an introduction 8-], where
those things are _highly discouraged_. Our coding standard does forbid
the first thing you do, and -since our coding standard does not say
anything about it- I would object against those last two changes.

Reason for the first thing not-to-do:

  if (some_test) {
        /* It's clear we have something to do here, */
        /* since we tested some_test */
        do_something();
  }

would become:

  if (some_test)
        /* It's clear we have something to do here, */
        /* since we tested some_test */
        do_something();

..now the error is easily made to add an statement, without adding the 
'}', especially when to comment gets large. So, only two versions
remain:

* the one-liner-without-{}:
  if (some_test) do_something();
* the non-one-liner-with-{}:
  if (some_test) {
        do_something();
  }

I think the for-rewriting is just painful, and obfuscates the code:
it's just like saying "goto's are forbidden on general
principle". Code should be formatted to make clear what is meant:
a for-loop over all indices with a return in the body makes exactly
clear what is meant, and can be understood (at least by me) in less
time than the other construction.

The same holds for replacing &tab[i] by tab+i: while tab+i is shorter, 
it is less clear what is meant by it.

> Others things I saw :
> 
>    - Lots of useless parenthesis in returns,
>    - Code lines that spread on more that one line. It breaks code
>    indentation,

...it breaks your editor? Or your indenter?

>    - FreeCiv is really a patchwork of code. All of them have differents
>    styles and indentations. It's really a bazaar.

...then use indent -c8 to format it to your needs.

The bazaar fits the development style, by the way :)

>    - Lots of new files ! :) goverment.c barbarian.c worklist.c 
>    that's cool


-- 
Rutger Nijlunsing, rutger @ null.net ----------------------------- Linux! --
Don't BiCapitalize without extremely good reason: it messes up the natural
human-eyeball search order -- Your Friendly Neighborhood Archive Maintainers
+31-40 ----------------------------------------------------------- ^X^S^X^Cs