[LC++]enum, #define, const, static const, or member const???

Carlo Wood carlo at alinoe.com
Sat Apr 6 00:15:04 UTC 2002


On Fri, Apr 05, 2002 at 09:23:09PM +0930, Mark Phillips wrote:
> > 1) Just use an integer
> > 
> > Bad idea, only do this the values are restricted to two values
> > (0 and 1; or -1 and 0), or to an infinite number of values
> > that can be devided into two or three 'kind' of groups, for
> > example: < 0, == 0, and > 0.  Where the actual value of the
> > int also has a meaning.  Not however that in that case you
> > are still not typing the literal number 2 (3, 4, ... etc).
> 
> But why bad?

With "Just use an integer" I mean (and assumed you did mean) code
like:

   int kind;

   if (kind == 3)
    ...
   else if (kind == 7)
    ...

Note the literal '3' and '7'.

Surely you understand why that is bad.

> My guess is because in C++ there is no way of
> saying "this is a special type of integer".  The way around this
> would be to do something like:
> 
> enum ShortRange {
>    srPredex=-1, srDeb=0,
>    srZero=0, srOne=1, srTwo=2,
>    srFin=2, srPodex=3};
> 
> Then you effectively get a subrange of the integers, with the
> advantages you mention above, __and__ you get your own separate
> type.  (The values srZero, srOne and srTwo are the "normal usage
> values", whereas srPredex and srPodex are "only to be used in special
> circumstances, like with index variables".)

First of all, this are enums, not literal integers.  So the "Bad idea"
phrase that I used doesn't refer to this case.  I had a seperate
section about enums.

That section stressed to use CLEAR names.  Surely "srZero", "srOne"
and "srTwo" are very bad names ;).  If that would be everything they
meant then you should use a normal integer of course.  For example:

bool do_teach(int number_of_pupils)
{
  if (number_of_pupils < 3)
    return false;
  return true;
}

Here the '3' means exactly that: the integer 3.
BAD again would be to write:

int main()
{
  ...

  if (number_of_pupils < 3)
     ...
  else
     ...
 
  ...


  if (number_of_pupils < 3)
     ...
  else
     ...

  ...
}

repeating the same value more than once.
You should use an (inlined) function in that case,
or use a constant:

int const minimum_number_of_pupils = 3;

int main()
{
  ...

  if (number_of_pupils < minimum_number_of_pupils)
     ...
  else
     ...
 
  ...


  if (number_of_pupils < minimum_number_of_pupils)
     ...
  else
     ...

  ...
}

> Below you seem to say that this method is good except not very
> maintainable.  Why?  Suppose I wanted to extend it to include
> -1 and 3 as "to be used" values.  I would just do:
> 
> enum ShortRange {
>    srPredex=-2, srDeb=-1
>    srMiOne=-1, srZero=0, srOne=1, srTwo=2, srThree=3,
>    srFin=3, srPodex=4
> };

And have srDeb == srMiOne ?
If { srMiOne, srZero, srOne, srTwo, srThree } are a subgroup
and { srDeb } is a subgroup, then these two shouldn't overlap,
that could cause confusion.  For example:

ShortRange data[] = { srTwo, srMiOne, srThree, srThree, srOne, srFin };

...

  for (int i = 0; data[i] != srFin; ++i)
    ..

would fail suddenly after you proposed addition.

> And as all for loops etc would be defined in terms of srPredex,
> srPodex, srDeb and srFin (possibly also in terms of srZero),
> code using these would not have to change.
> 
> (By the way, "predex" is short for "pre-index-set", "podex" is short
> for "post-index-set", "deb" is short for "debut index", and "fin" is
> short for "finale index".

I used it as "Finish" above ;)

> Whereas srPredex and srPodex should be
> thought of as "special" elements distinct from the usual range,
> srDeb and srFin are simply alternative labels for the first and last
> elements respectively of the usual range.)

As I described in my previous post, you can do that.
Let me give an example of code that I wrote myself
(from http://libcwd.sourceforge.net)

// The different instance-ids used in libcwd.
enum mutex_instance_nt {
  // Recursive mutexes.
  tsd_initialization_instance,
  object_files_instance,        // rwlock
  end_recursive_types,
  // Fast mutexes.
  memblk_map_instance,
  mutex_initialization_instance,
  ids_singleton_tct_S_ids_instance,
  alloc_tag_desc_instance,
  type_info_of_instance,
  dlopen_map_instance,
  write_max_len_instance,
  set_ostream_instance,
  debug_objects_instance,       // rwlock
  debug_channels_instance,      // rwlock
  // Values reserved for read/write locks.
  reserved_instance_low,
  reserved_instance_high = 3 * reserved_instance_low,
  // Values reserved for test executables.
  test_instance0 = reserved_instance_high,
  test_instance1,
  test_instance2,
  test_instance3,
  instance_locked_size          // Must be last in list
};


As you can see, I have devided the enum values in different sub-groups
again (seperated with the comments).

Then I wrote code like:

template <int instance>
  class rwlock_tct {
...
    static void rdunlock(void) throw()
    {
      if (instance < end_recursive_types && pthread_equal(S_writer_id, pthread_self()))
        return;                                         // No error checking is done.
      S_no_holders_condition.lock();
      if (--S_holders_count == 0)                       // Was this the last reader?
        S_no_holders_condition.signal();                // Tell waiting threads.
      S_no_holders_condition.unlock();
    }

As a result, rwlock_tct<object_files_instance> will do the needed
test 'pthread_equal(S_writer_id, pthread_self())' in rdunlock()
in order to make it recursive.  While the other lock instances
(ie rwlock_tct<debug_objects_instance>::rdunlock) will be faster,
not doing the check, but are also not recursive).

> > 2) Using macros for a constant
> > 
> > Never do that.
> 
> 
> Why not?  Is it because the compiler is given no type
> information and may have limited opportunity for checking?
> ...whereas doing
> 
> int const myConstant=6;
> 
> does everything a #define does, but does type checking
> as well.

yep.  There are (much) better alternatives for macros in most
cases.  When there are you should use those.

> > Disadvantages:
> > - If the kinds have more grouping in common it is not
> >   really supported to the actual value of the enums
> >   to compare (ie, < 0, == 0, > 0).  You can assign the
> >   integer values to the enums but it becomes less
> >   maintainable.
> > - You can't have a group like mentioned in the previous
> >   point with an infinite number of members.
> > - You can't treat the values also as integers (ie, array
> >   index) because that is very dangerous and you are not
> >   allowed to do arithmetics in certain cases.
> 
> 
> With the setup I used with my "ShortRange" type, surely it
> would be safe to use them like integers?

Well, no.  But lets say you need them as array index, then 
you clearly can't change them later (or you'd need to make
sure that all arrays put the corresponding elements in the
right place and that is not maintainable).

Then, I suppose, you could do something like you did.
You shouldn't use the same values for two different elements
though :/.  Also, it is must more likely that each element
of the array has a special meaning.  Can't you use names
that reflect THAT meaning?  And if they don't, then why do
you need to pass the index around?

Let me make up an example.

enum PieceThatGivesCheck {
  ckNone = -1,
  ckPawn = 0,
  ckBishop = 1,
  ckKnight = 2,
  ckRook = 3,
  ckQueen = 4
};

and you use 'Pawn' till 'Queen' as array index.
Then it is likely you will have an array with:

PieceCharacteristics pieces[] = { ... };

and another enum with:

enum Piece {
  Pawn,
  Bishop,
  Knight,
  Rook,
  Queen,
  King
};

Obviously you then should rewrite the first enum as:

enum PieceThatGivesCheck {
  ckNone = -1,
  ckPawn = Pawn,
  ckBishop = Bishop,
  ckKnight = Knight,
  ckRook = Rook,
  ckQueen = Queen
};

Do we still feel sure that ckNone has a unique value?

Now later you need to make a distinction between a
normal pawn that gives check and a pawn that gives
check but can be taken 'en passant'...

enum PieceThatGivesCheck {
  ckNone = -1,
  ckPawn = Pawn,
  ckBishop = Bishop,
  ckKnight = Knight,
  ckRook = Rook,
  ckQueen = Queen
  ckPawnEnPassant = Queen + 1;
};

Brrrr, horrible.  Now ckPawnEnPassant == King.
Better would be to add PawnEnPassant to enum Piece
too, but you don't want to duplicate the data
in the array that THOSE enums are refering to...
the 'enpassant' bit is just extra information.

At some point you just have to stop putting data
into enums and choose a clear design like:

// Array indexes
int const Pawn = 0;
int const Bishop = 1;
int const Knight = 2;
int const Rook = 3;
int const Queen = 4;
int const King = 5;

Piece pieces[] = { { { -1, 1 }, { -1, -1 }, ... }, 5, ... };	// data for the pieces

struct PieceThatGivesCheck {
  Piece const& piece;
  bool canBeTakenEnPassant;
};

in this case you can clearly extent PieceThatGivesCheck
all you want without any danger that you'd screw up
code elsewhere.

Personally I only use enums for something that literally means
"instance" of something and preferably not as array index,
it is possible though I just don't like the feeling.
I DO use the fact that enums are ordered, so you can savely
know that blah < foo, and will STAY smaller independent of
the additions made later.  That means you can make at least
two subgroups and often three or more, depending on their
mutual relationship.  It also allows you to use 'markers'
that signify the start/end of a subgroup.
The problem with indexes is that they are ALL a subgroup
on their own AND need a fixed value: you can't insert
a value later without shifting data in all arrays that use
them, that is bad.

> > 5) Using 'static int const'
> > 
> > Doesn't polute the global namespace.  You want this when
> > the values are completely related to that particular
> > class (as in your case).  The enum definition should also
> > be put inside the class for the same reason: not to polute
> > global namespace (I'd even make it private when possible).
> 
> Isn't "static int const" equivalent to "int const"?  Ie doesn't
> the latter default to static?

no.  If your compiler does that then it is non conforming to ISO C++.

int const x = 3;		// Exported.
static int const y = 4;		// Not exported.

struct A {
  int z;			// Accessed as a.z
  int const x = 3;		// Syntax error.
  static int const y;		// Exported. Accessed as A::y
};

int const A::y = 4;

-- 
Carlo Wood <carlo at alinoe.com>




More information about the tuxCPProgramming mailing list