Re: [lmi] Nearly duplicative rate-table enums

[Vadim Zeitlin]

On Wed, 7 Feb 2018 17:11:32 +0000 Greg Chicares <address@hidden> wrote:

GC> Vadim--'rate_table.cpp' has these two enums:
GC> 
GC> // This enum defines the indices of all the known fields in soa_fields 
array,
GC> // its elements are consecutive.
GC> enum enum_soa_field
GC>     {e_field_table_name
GC>     ,e_field_table_number
GC> ...
GC>     ,e_field_hash_value
GC>     };
GC> 
GC> // This enum defines the field record types used in the binary SOA format 
and
GC> // exists mostly to allow writing a switch on the record type in a readable 
way.
GC> enum
GC>     {e_record_table_name          =  1
GC>     ,e_record_table_number        =  2
GC> ...
GC>     ,e_record_hash_value          = 18
GC>     ,e_record_end_table           = 9999
GC>     };
GC> 
GC> ...which are almost equivalent, except:
GC>  - one has 'field' in its enumerators; the other has 'record'
GC>  - one has a name; the other is anonymous
GC>  - one's enumerators are defaulted; the other has explicit values
GC>  - only one includes an end-of-table enumerator
GC> 
GC> Why do it that way, instead of combining those into a single enum
GC> with the union of all the useful features of both?

 The 2 enums are necessary because I wanted to be able to use symbolic
names as indices into soa_fields (this is the first one, whose values must
necessarily be consecutive because of this) and I also needed symbolic
names for the values actually used in the SOA files (this is the second
one, whose values are fixed).

 This provides answers to the questions above:

- One identifies "fields" in the program code, the other identifies
  "records" in the external files.
- It makes sense to have a variable of enum_soa_field type, but not
  really of the (non-existent) enum_soa_record type, see below.
- One uses default values to ensure that they're consecutive, the
  other one can not.
- The end table element corresponds to the actual value used in the
  data file, but we don't actually have any field with this value (because
  there is no associated value). We could still add e_field_end, I guess,
  but it wouldn't be actually used anywhere except, perhaps, in a
  static_assert (which I didn't have at my disposal when writing this code
  originally) checking sizeof(soa_fields).


GC> The last two 'switch' statements are the only ones that use these
GC> enums. The switch condition is of enumerative type in the last
GC>     switch(field)

 In this case, "field" is known to be a valid enum element, so there is no
problem with doing this.

GC> but not in the second-to-last
GC>     switch(record_type)

 In this case, "record_type" is a 16 bit value read from a data file and so
may contain a completely arbitrary value. It would be a bad idea to pretend
that it's always one of the e_record_xxx values because this is not
actually true.

GC> and I wonder whether that "switch(record_type)" would be better as
GC>     switch(static_cast<SOMETHING>(record_type))

 So IMO this would definitely be a bad idea.

GC> where we'd have to give a name to the anonymous enum.

 In fact, to complete the answer to the second question above, this enum is
anonymous because there is no corresponding type, it's really just a
collection of named values.

GC> why not just merge the enums?

 I don't see any way to merge them because of the constraints enumerated
above. The only idea to avoid duplicating their names that I had was to use
a macro (or X-macros), but it didn't seem to be worth it.

 Hope this explains why I did it like this, but please let me know if you
have any further questions,
VZ