[Freeciv-Dev] Re: Government file.

[Falk Hueffner <falk.hueffner@xxxxxxxxxxxxxxxxxxxxxxxx>]

Nicolas BRUNEL <brunel@xxxxxxxxxxxxxxxxxxxx> writes:

> Hello,
> 
>    I had got some free time to modify common/goverment.c .
> 
>    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;

Well, I find the first version much more readable. It is immediately
obvious that it is a simple iteration loop, and you can then look what
is actually done in the loop body. Also, empty loop bodies are
confusing.

>    - & Tab[i] is equivalent to Tab+i .
>      IMHO, as it is shorter, Tab+i is better.

Sure it is equivalent, but the latter relies on the strange pointer
arithmetic semantics of C with which many people are not familiar. I
think the first version is clearer. Or do you really think

title = gov->ruler_titles+gov->num_ruler_titles-1;

is easier to understand than

title = &(gov->ruler_titles[gov->num_ruler_titles-1]);

?

With regard to braces, I think it's confusing to use braces in one
branch of an if...else... and not in the other.

Generally, I would optimize the code not for shortness, but clarity.
It should be obvious even to readers who don't know every dusty corner
of C what the code does.


        Falk