C++ – Explicitly define enum values, even if the default value is the same

ccode-qualitycode-reviewsenum

There are times when an enum's values are important: it is not necessary for them to be unique, they also need to have specific values. In such cases, should the values be explicitly defined, even if they coincide with the defaults? An example:

enum Car {
    DeLorean = 0,
    Lada = 1
};

Imagine that for whatever reason your application assumes that DeLorean and Lada have those specific values. Incidentally, they are the same as the default values but does that mean it is no longer necessary to use explicit definitions?

Leaving them implicit makes me uneasy. It seams to me that having them explicitly defined is communicating to future programmers that the specific values are important and it helps prevent mistakes like this:

enum Car {
    Dacia,
    DeLorean,
    Lada
};

In the example above, another programmer who is not aware of the restriction I mentioned introduces a new enum value and, wanting to keep the code tidy, puts it in alphabetical order. However, DeLorean and Lada now have different numerical values and a potentially silent bug has been introduced.

My reasoning seems correct to me (obviously), but the code review team for a company I used to work with didn't agree. Am I wrong?

Best Answer

This is very much a judgement call, but I would agree with you that explicit values are the way to go, plus a comment at the start explaining the significance of the explicit values.

As an example of a prominent project that is doing it this way, look at Clang's AST serialization. It uses a huge enum to define codes for all of its AST nodes, and the enum has explicit values.

That said, I also sympathize with the opinion of the architecture team that touching old code that works when there's more important things to be done is not the best idea.

I don't like the compromise solution, though. It adds nothing.

Related Solutions

What are the common misuses of “enum” in C

One case I can think of, though haven't seen in the wild yet, is to abuse enums as a collection of disjointed integer constants:

enum somestuff { days_in_week = 7,
                 months_in_year = 12,
                 number_of_planets = 8,
                 fingers_per_hand = 5 };

The most pathological, innocent looking example I can imagine would be

enum numbers { one, two, three, four, five, six, seven, eight, nine, ten };

because one==0, two==1 etc.

Java – Several enum classes that declare constants with the same names

This can get confusing... and Magic The Gathering is one that loves to do exceptions to rules. Trying to codify them can often be a challenge.

On the "MonoColored Hybrid" read this and this. And then this for Phyrexian mana.

However, this can boil down to something that is rather sensible.. and doesn't stick too much in a single enum or confuse the next person with an inheritance tree of enums. Enums really 'want' to be simple things, so let them be so. The simpler they are, the easier they are to use, and its a good thing to use enums.

A card has some cost. The good old fashioned lighting bolt is just "R". This is a List of some sort. It could even be empty as in the case of 0 cost artifacts (unless you want to do null object). But its a List.

Each element of the list is a SpellCost object.

A SpellCost would then be composed of:

An EnumMap of costs. The Enum that backs the map would be:

enum Mana {
  WHITE,
  BLUE,
  BLACK,
  RED,
  GREEN,
  COLORLESS
}

And then you would have the EnumMap be as a EnumMap<Mana, Integer>. Something that was a "R/2" for "Red or two colorless" could then be specified in the EnumMap as: mana.put(Mana.RED,1); mana.put(Mana.COLORLESS,2); This also handles cards that are described as "R/W G" (such as Naya Hushblade - because not all cards are monocolored hybrids).

^{_{The EnumMap and EnumSet are two of forgotten collections that are really nice once you realize what they are and do. They give you type safe entries in a map and are backed by either a simple array or a bitfield. The type safety on it can prevent a number of bugs associated with bitfields. And the type safety on the map is something to think about whoever you've got a Map<String, Object> and wonder what happens when there's a typo in the String.}}

Phyrexian mana is then an attribute of the SpellCost. Its a int (though you could do it as a boolean later and yell YAGNI in my general direction). Currently the definition of the cost is "mana or two life" but I wouldn't count on that being the case forever and locking yourself into a boolean now... well.

So, you've got:

public class SpellCost {
    EnumMap<Mana, Integer> mana;
    int lifeCost;

    ...
}

And then write the associated methods for it. You might want to write a container for the cost itself rather than leaving it as a bare List so that you can have appropriate methods against it.

The key design points for this is that it keeps the enum simple as enums aught to be. Any of the complexity of the business logic is kept within the SpellCost class (and possibly the container class too).

Best Answer

Related Solutions

What are the common misuses of “enum” in C

Java – Several enum classes that declare constants with the same names

Related Topic