[c++] Should I use #define, enum or const?



6 Answers

Forget the defines

They will pollute your code.

bitfields?

struct RecordFlag {
    unsigned isnew:1, isdeleted:1, ismodified:1, isexisting:1;
};

Don't ever use that. You are more concerned with speed than with economizing 4 ints. Using bit fields is actually slower than access to any other type.

However, bit members in structs have practical drawbacks. First, the ordering of bits in memory varies from compiler to compiler. In addition, many popular compilers generate inefficient code for reading and writing bit members, and there are potentially severe thread safety issues relating to bit fields (especially on multiprocessor systems) due to the fact that most machines cannot manipulate arbitrary sets of bits in memory, but must instead load and store whole words. e.g the following would not be thread-safe, in spite of the use of a mutex

Source: http://en.wikipedia.org/wiki/Bit_field:

And if you need more reasons to not use bitfields, perhaps Raymond Chen will convince you in his The Old New Thing Post: The cost-benefit analysis of bitfields for a collection of booleans at http://blogs.msdn.com/oldnewthing/archive/2008/11/26/9143050.aspx

const int?

namespace RecordType {
    static const uint8 xNew = 1;
    static const uint8 xDeleted = 2;
    static const uint8 xModified = 4;
    static const uint8 xExisting = 8;
}

Putting them in a namespace is cool. If they are declared in your CPP or header file, their values will be inlined. You'll be able to use switch on those values, but it will slightly increase coupling.

Ah, yes: remove the static keyword. static is deprecated in C++ when used as you do, and if uint8 is a buildin type, you won't need this to declare this in an header included by multiple sources of the same module. In the end, the code should be:

namespace RecordType {
    const uint8 xNew = 1;
    const uint8 xDeleted = 2;
    const uint8 xModified = 4;
    const uint8 xExisting = 8;
}

The problem of this approach is that your code knows the value of your constants, which increases slightly the coupling.

enum

The same as const int, with a somewhat stronger typing.

typedef enum { xNew = 1, xDeleted, xModified = 4, xExisting = 8 } RecordType;

They are still polluting the global namespace, though. By the way... Remove the typedef. You're working in C++. Those typedefs of enums and structs are polluting the code more than anything else.

The result is kinda:

enum RecordType { xNew = 1, xDeleted, xModified = 4, xExisting = 8 } ;

void doSomething(RecordType p_eMyEnum)
{
   if(p_eMyEnum == xNew)
   {
       // etc.
   }
}

As you see, your enum is polluting the global namespace. If you put this enum in an namespace, you'll have something like:

namespace RecordType {
   enum Value { xNew = 1, xDeleted, xModified = 4, xExisting = 8 } ;
}

void doSomething(RecordType::Value p_eMyEnum)
{
   if(p_eMyEnum == RecordType::xNew)
   {
       // etc.
   }
}

extern const int ?

If you want to decrease coupling (i.e. being able to hide the values of the constants, and so, modify them as desired without needing a full recompilation), you can declare the ints as extern in the header, and as constant in the CPP file, as in the following example:

// Header.hpp
namespace RecordType {
    extern const uint8 xNew ;
    extern const uint8 xDeleted ;
    extern const uint8 xModified ;
    extern const uint8 xExisting ;
}

And:

// Source.hpp
namespace RecordType {
    const uint8 xNew = 1;
    const uint8 xDeleted = 2;
    const uint8 xModified = 4;
    const uint8 xExisting = 8;
}

You won't be able to use switch on those constants, though. So in the end, pick your poison... :-p

Question

In a C++ project I'm working on, I have a flag kind of value which can have four values. Those four flags can be combined. Flags describe the records in database and can be:

  • new record
  • deleted record
  • modified record
  • existing record

Now, for each record I wish to keep this attribute, so I could use an enum:

enum { xNew, xDeleted, xModified, xExisting }

However, in other places in code, I need to select which records are to be visible to the user, so I'd like to be able to pass that as a single parameter, like:

showRecords(xNew | xDeleted);

So, it seems I have three possible appoaches:

#define X_NEW      0x01
#define X_DELETED  0x02
#define X_MODIFIED 0x04
#define X_EXISTING 0x08

or

typedef enum { xNew = 1, xDeleted, xModified = 4, xExisting = 8 } RecordType;

or

namespace RecordType {
    static const uint8 xNew = 1;
    static const uint8 xDeleted = 2;
    static const uint8 xModified = 4;
    static const uint8 xExisting = 8;
}

Space requirements are important (byte vs int) but not crucial. With defines I lose type safety, and with enum I lose some space (integers) and probably have to cast when I want to do a bitwise operation. With const I think I also lose type safety since a random uint8 could get in by mistake.

Is there some other cleaner way?

If not, what would you use and why?

P.S. The rest of the code is rather clean modern C++ without #defines, and I have used namespaces and templates in few spaces, so those aren't out of question either.




Based on KISS, high cohesion and low coupling, ask these questions -

  • Who needs to know? my class, my library, other classes, other libraries, 3rd parties
  • What level of abstraction do I need to provide? Does the consumer understand bit operations.
  • Will I have have to interface from VB/C# etc?

There is a great book "Large-Scale C++ Software Design", this promotes base types externally, if you can avoid another header file/interface dependancy you should try to.




Even if you have to use 4 byte to store an enum (I'm not that familiar with C++ -- I know you can specify the underlying type in C#), it's still worth it -- use enums.

In this day and age of servers with GBs of memory, things like 4 bytes vs. 1 byte of memory at the application level in general don't matter. Of course, if in your particular situation, memory usage is that important (and you can't get C++ to use a byte to back the enum), then you can consider the 'static const' route.

At the end of the day, you have to ask yourself, is it worth the maintenance hit of using 'static const' for the 3 bytes of memory savings for your data structure?

Something else to keep in mind -- IIRC, on x86, data structures are 4-byte aligned, so unless you have a number of byte-width elements in your 'record' structure, it might not actually matter. Test and make sure it does before you make a tradeoff in maintainability for performance/space.







Do you actually need to pass around the flag values as a conceptual whole, or are you going to have a lot of per-flag code? Either way, I think having this as class or struct of 1-bit bitfields might actually be clearer:

struct RecordFlag {
    unsigned isnew:1, isdeleted:1, ismodified:1, isexisting:1;
};

Then your record class could have a struct RecordFlag member variable, functions can take arguments of type struct RecordFlag, etc. The compiler should pack the bitfields together, saving space.




I would rather go with

typedef enum { xNew = 1, xDeleted, xModified = 4, xExisting = 8 } RecordType;

Simply because:

  1. It is cleaner and it makes the code readable and maintainable.
  2. It logically groups the constants.
  3. Programmer's time is more important, unless your job is to save those 3 bytes.



Enums would be more appropriate as they provide "meaning to the identifiers" as well as type safety. You can clearly tell "xDeleted" is of "RecordType" and that represent "type of a record" (wow!) even after years. Consts would require comments for that, also they would require going up and down in code.






Related