[Freeciv-Dev] (PR#11477) gen imprs

["Vasco Alexandre da Silva Costa" <vasc@xxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=11477 >

> [jdorje - Fri Dec 17 08:37:28 2004]:
> 
> I don't see any technical problems.  But there is room for stylistic
> discussion.
> 
> - Some style errors / could use more comments.

I fixed some of these in the latest iteration of the patch. If there are
any left, please say so.

> - Rather than rename global wonders as great wonders and adding small
> wonders, why not just leave the original name and call them player
wonders?

I did this for several reasons. One is that for people which played
Civilization III these names are familiar, while if we picked our own
names everyone would have to get re-acquainted with them. Another reason
was that changing the pre-existing name allowed me to easily catch any
missing places in the code that I needed to change.
 
> - The names of can_sell_improvement and can_sell_building are too
> indistinguishable.  I think they should be can_sell_building and
> can_city_sell_building.

Ah, well you know how I feel about this. I think we should call the most
generic class 'buildings' and the subclass 'improvements'. But I didn't
want to engage in a massive renaming right now. It would hide the actual
implementation made by the patch.

> - is_improvement() isn't clear enough.  All improvements are
> improvements.  I think this should be is_normal_improvement().

Until we don't do the massive renaming these problems will always happen.

> - "genus" is an okay name but this is a new term in freeciv.  Don't we
> have concepts like this elsewhere?  What names do we use there?  I think
> this corresponds most closely to move_type in unittype.h, but no
> corresponding name really comes to mind.

I thought about calling it 'class', because it was used elsewhere, but
since that is a C++ no-no I chose something similar, with about the same
word length. There was already an 'impr_type' and calling it
'building_type' would class with a possible future rename.