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

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Wed, Feb 13, 2002 at 10:54:55AM CET, I got a letter,
where Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> told me, that...
> 
> There is some code like
> ./common/unittype.c:368:  for (i = 0; i < UCL_LAST; i++) {
> ./common/unittype.c:386:  for(i=0; i<F_LAST; i++) {
> ./common/unittype.c:515:  for(i=0; i<F_LAST; i++) {
> ./common/unittype.c:506:    for(i=0; i<L_LAST; i++) {
> ./common/unittype.c:510:  for(i=0; i<L_LAST; i++) {
> ./common/improvement.c:143:  for (ret_id = 0; ret_id < EFR_LAST; ret_id++) {
> ./common/improvement.c:176:  for (ret_id = 0; ret_id < EFT_LAST; ret_id++) {
> ./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 X_FIRST and X_LAST has to be part of the enum and not integers. So
> things like:
>   #define L_FIRST 64              /* should be >= F_LAST */
>   #define B_LAST MAX_NUM_ITEMS
> have to go.

Yes.

> Another thing: 
> 
> enum effect_range_id {
>   EFR_NONE,
>   EFR_BUILDING,
>   EFR_CITY,
>   EFR_ISLAND,
>   EFR_PLAYER,
>   EFR_WORLD,
>   EFR_LAST      /* keep this last */
> };
> 
> typedef enum effect_range_id Eff_Range_id;
> 
> Should be replaced with
> 
> typedef enum {
>   EFR_NONE,
>   EFR_BUILDING,
>   EFR_CITY,
>   EFR_ISLAND,
>   EFR_PLAYER,
>   EFR_WORLD,
>   EFR_LAST      /* keep this last */
> } Eff_Range_id;
> 
> Since the named enum is never used in the code.

Yes. I would in fact prefer this way for all enums at all.

> 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.

Yes. I've already met this in my cleanups, IIRC.

-- 

                                Petr "Pasky" Baudis

* UN*X programmer && admin         * IPv6 guy (XS26 co-coordinator)
* elinks maintainer                * FreeCiv AI hacker
* IRCnet operator
.
I love deadlines.
I love the whooshing sound they make as they fly by.
-- Douglas Adams.
.
Public PGP key && geekcode && homepage: http://pasky.ji.cz/~pasky/