35. Adding a new constant to enum don't forget to correct switch operators

The fragment is taken from the Appleseed project. The code contains an error that PVS-Studio analyzer diagnoses in the following way: V719 The switch statement does not cover all values of the 'InputFormat' enum: InputFormatEntity.

enum InputFormat
{
    InputFormatScalar,
    InputFormatSpectralReflectance,
    InputFormatSpectralIlluminance,
    InputFormatSpectralReflectanceWithAlpha,
    InputFormatSpectralIlluminanceWithAlpha,
    InputFormatEntity
};

switch (m_format)
{
  case InputFormatScalar:
    ....
  case InputFormatSpectralReflectance:
  case InputFormatSpectralIlluminance:
    ....
  case InputFormatSpectralReflectanceWithAlpha:
  case InputFormatSpectralIlluminanceWithAlpha:
    ....
}

Explanation

Sometimes we need to add a new item to an existing enumeration (enum), and when we do, we also need to proceed with caution - as we will have to check where we have referenced the enum throughout all of our code, e.g., in every switch statement and if chain. A situation like this can be seen in the code given above.

InputFormatEntity was added to the InputFormat - I'm making that assumption based on the fact that the constant has been added to the end. Often, programmers add new constants to the end of enum, but then forget to check their code to make sure that they've dealt with the new constant properly throughout, and corrected the switch operator.

As a result we have a case when "m_format==InputFormatEntity" isn't handled in any way.

Correct code

switch (m_format)
{
  case InputFormatScalar:
  ....
  case InputFormatSpectralReflectance:
  case InputFormatSpectralIlluminance:
  ....
  case InputFormatSpectralReflectanceWithAlpha:
  case InputFormatSpectralIlluminanceWithAlpha:
  ....
  case InputFormatEntity:
  ....
}

Recommendation

Let's think, how can we reduce such errors through code refactoring? The easiest, but not a very effective solution is to add a "default:", that will cause a message to appear, e.g.:

switch (m_format)
{
  case InputFormatScalar:
  ....
  ....
  default:
    assert(false);
    throw "Not all variants are considered"
}

Now if the m_format variable is InputFormatEntity, we'll see an exception. Such an approach has two big faults:

1. As there is the chance that this error won't show up during testing (if during the test runs, m_format is not equal to InputFormatEntity), then this error will make its way into the Release build and would only show up later - during runtime at a customer's site. It's bad if customers have to report such problems!

2. If we consider getting into default as an error, then you have to write a case for all of the enum's possible values. This is very inconvenient, especially if there are a lot of these constants in the enumeration. Sometimes it's very convenient to handle different cases in the default section.

I suggest solving this problem in the following way; I can't say that it's perfect, but at least it's something.

When you define an enum, make sure you also add a special comment. You can also use a keyword and an enumeration name.

Example:

enum InputFormat
{
  InputFormatScalar,
  ....
  InputFormatEntity
  //If you want to add a new constant, find all ENUM:InputFormat.
};

switch (m_format) //ENUM:InputFormat
{
  ....
}

In the code above, when you change the InputFormat enum, you are directed to look for "ENUM:InputFormat" in the source code of the project.

If you are in a team of developers, you would make this convention known to everybody, and also add it to your coding standards and style guide. If somebody fails to follow this rule, it will be very sad.