[Freeciv-Dev] Re: [RFC] enum cleanup

[Ben Webb <ben@xxxxxxxxxxxxxxxxxxxxxx>]

On Wed, 13 Feb 2002, Raimar Falke wrote:

> ./common/improvement.c:302:  for (i=0;i<EFR_LAST;i++) implist[i]=NULL;
> ./common/improvement.c:338:    for (i=0;i<EFR_LAST;i++) {
> ./common/improvement.c:350:      for (i=0;i<EFR_LAST;i++) {
> ./common/improvement.c:547:  for (i=0; i<EFR_LAST; i++)
> ./common/improvement.c:560:  for (j=0; j<EFR_LAST; j++) {
> 
> To be type safe this has to be replaced with:
>   for(i = X_FIRST; i < X_LAST; i++)

        The only problem with this is that there isn't an EFR_FIRST 
defined, rather EFR_NONE. This of course then creates problems if the 
order of EFR_XXX is changed such that EFR_NONE is no longer first.

What about
typedef enum {
  EFR_FIRST,    /* keep this first */
  EFR_NONE = EFR_FIRST,
  EFR_BUILDING,
  EFR_CITY,
  EFR_ISLAND,
  EFR_PLAYER,
  EFR_WORLD,
  EFR_LAST      /* keep this last */
} Eff_Range_id;

? Ugly, but it should avoid that problem. (On the other hand, EFR_LAST 
isn't really the "last" element - it's the one after the last. But I guess 
EFR_ONEAFTERLAST would look even more ugly.)

> And last but not least: there is
>   typedef int Impr_Type_id;
> which is used in the code. And there is
> enum improvement_type_id {
>   B_AIRPORT=0, B_AQUEDUCT, B_BANK, B_BARRACKS, B_BARRACKS2, B_BARRACKS3,
>   B_CATHEDRAL, B_CITY, B_COASTAL, B_COLOSSEUM, B_COURTHOUSE,  B_FACTORY,
>   B_GRANARY, B_HARBOUR, B_HYDRO, B_LIBRARY, B_MARKETPLACE, B_MASS, B_MFG,
>   B_NUCLEAR, B_OFFSHORE, B_PALACE, B_POLICE, B_PORT, B_POWER,
>   B_RECYCLING, B_RESEARCH, B_SAM, B_SDI, B_SEWER, B_SOLAR, B_SCOMP,
>   B_SMODULE, B_SSTRUCTURAL, B_STOCK, B_SUPERHIGHWAYS, B_SUPERMARKET, B_TEMPLE,
>   B_UNIVERSITY,
> 
>   B_APOLLO, B_ASMITHS, B_COLLOSSUS, B_COPERNICUS, B_CURE, B_DARWIN, B_EIFFEL,
>   B_GREAT, B_WALL, B_HANGING, B_HOOVER, B_ISAAC, B_BACH, B_RICHARDS,
>   B_LEONARDO, B_LIGHTHOUSE, B_MAGELLAN, B_MANHATTEN, B_MARCO, B_MICHELANGELO,
>   B_ORACLE, B_PYRAMIDS, B_SETI, B_SHAKESPEARE, B_LIBERTY, B_SUNTZU,
>   B_UNITED, B_WOMENS,
>   B_CAPITAL, B_LAST_ENUM
> };
> 
> These two should be merged into
> 
> typedef enum {...} Impr_Type_id.

        I disagree. The enum improvement_type_id will (hopefully) be 
completely redundant in the near future, and so will be able to be removed 
entirely, so we end up with just "typedef int Impr_Type_id;" Rolling the 
enum into the Impr_Type_id just makes impr_gen harder in the long term.

        Ben
-- 
ben@xxxxxxxxxxxxxxxxxxxxxx           http://bellatrix.pcl.ox.ac.uk/~ben/
"And do you know Carolina where the biscuits are soft and sweet?"