[Freeciv-Dev] Re: [RFC] enum cleanup
[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
On Wed, Feb 13, 2002 at 02:42:20PM +0000, Ben Webb wrote:
> 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.
government.h has:
enum government_flag_id {
G_BUILD_VETERAN_DIPLOMAT=0, /* and Spies (in general: all F_DIPLOMAT) */
...
G_LAST_FLAG
};
#define G_FIRST_FLAG G_BUILD_VETERAN_DIPLOMAT
So *_FIRST is a define. I would prefer this.
> (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.)
Yes I also noticed this.
> > 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.
I don't see the problem. The code will now and later use
"Impr_Type_id". The code uses now B_* which should be an element of
Impr_Type_id.
Raimar
--
email: rf13@xxxxxxxxxxxxxxxxx
"This is Linux Country. On a quiet night, you can hear Windows reboot."
|
|