[Freeciv-Dev] Re: Government file.
[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
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
|
|