Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2000:
[Freeciv-Dev] Re: Government file.
Home

[Freeciv-Dev] Re: Government file.

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: Government file.
From: Falk Hueffner <falk.hueffner@xxxxxxxxxxxxxxxxxxxxxxxx>
Date: 21 Jan 2000 11:29:29 +0100

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

[Prev in Thread] Current Thread [Next in Thread]