Mikhail
Mikhail

Reputation: 21749

Enum mapping robust to refactoring

I want to map values of a (scoped) enum to some other values. For instance, here I map Color to other enum Group:

enum class Color {
  Red, Green, Blue, Cyan, Magenta, Yellow, White, Black, 
  COUNT  // Needed to know the number of items
};
enum class Group {
  Primary, Secondary, Neutral
};

Group GetGroupOfColor(Color color);  // Mapping function I'm going to implement

I want to make sure that if anyone changes the number of elements in Color enum, this function will fail to compile.

I came up with the only solution to this problem:

Group GetGroupOfColor(Color color)
{
  static const Group color2group[] = {
    Group::Primary,    // Red
    Group::Primary,    // Green
    Group::Primary,    // Blue
    Group::Secondary,  // Cyan
    Group::Secondary,  // Magenta
    Group::Secondary,  // Yellow
    Group::Neutral,    // White
    Group::Neutral     // Black
  };
  static_assert(ARRAY_SIZE(color2group) == size_t(Color::COUNT), "DEADBEEF!");

  auto index = size_t(color);
  assert(index < size_t(Color::COUNT));
  return color2group[index];
}

where ARRAY_SIZE may be implemented like the following:

template <typename T, size_t N>
constexpr size_t ARRAY_SIZE(T(&)[N])
{
  return N;
}

This implementation does what I want, but it has a bunch of drawbacks:

My question is, is there a way to improve this implementation? Maybe some different approach I did not even think about. Maybe with its own drawbacks, which I will find less annoying.

See also:

Upvotes: 2

Views: 410

Answers (3)

Frerich Raabe
Frerich Raabe

Reputation: 94329

I suspect you'd like to detect changes to the Color enum because you want to ensure that every color can always be mapped to some group? If so, you could establish the mapping using some macros which make it impossible to add a new Color value without also defining what it maps to. Something like:

#define COLORMAP \
  MAP_COLOR(Red, Primary) \
  MAP_COLOR(Green, Primary) \
  MAP_COLOR(Blue, Primary) \
  MAP_COLOR(Cyan, Secondary) \
  MAP_COLOR(Magenta, Secondary) \
  MAP_COLOR(Yellow, Secondary) \
  MAP_COLOR(White, Neutral) \
  MAP_COLOR(Black, Neutral)

Defining the MAP_COLOR appropriately will then allow you to define the Color enum as well as the mapping function from a single source:

#define MAP_COLOR(a, b) a,
enum class Color {
  COLORMAP
  Invalid
};
#undef MAP_COLOR

enum class Group {
  Primary, Secondary, Neutral, Invalid
};

#define MAP_COLOR(a, b) case Color::a: return Group::b;
Group GetGroupOfColor(Color color)
{
  switch (color) {
    COLORMAP
    case Color::Invalid: return Group::Invalid;
  }
  return Group::Invalid;
}

Upvotes: 1

Sander De Dycker
Sander De Dycker

Reputation: 16243

How about a slightly different approach - create a kind of enriched enum :

class Color {
  public :
    enum class Name { RED, GREEN, BLUE, CYAN, MAGENTA, YELLOW, WHITE, BLACK };
    enum class Group { PRIMARY, SECONDARY, NEUTRAL };

    uint32_t rgb;
    Name name;
    Group group;

    static const Color Red;
    static const Color Green;
    static const Color Blue;
    static const Color Cyan;
    static const Color Magenta;
    static const Color Yellow;
    static const Color White;
    static const Color Black;

  private :
    Color(uint32_t rgb, Name name, Group group) : rgb(rgb), name(name), group(group) { }

  public :
    inline operator const Name() const { return name; }
};

const Color Color::Red     = Color(0xFF0000, Color::Name::RED,     Color::Group::PRIMARY);
const Color Color::Green   = Color(0x00FF00, Color::Name::GREEN,   Color::Group::PRIMARY);
const Color Color::Blue    = Color(0x0000FF, Color::Name::BLUE,    Color::Group::PRIMARY);
const Color Color::Cyan    = Color(0x00FFFF, Color::Name::CYAN,    Color::Group::SECONDARY);
const Color Color::Magenta = Color(0xFF00FF, Color::Name::MAGENTA, Color::Group::SECONDARY);
const Color Color::Yellow  = Color(0xFFFF00, Color::Name::YELLOW,  Color::Group::SECONDARY);
const Color Color::White   = Color(0xFFFFFF, Color::Name::WHITE,   Color::Group::NEUTRAL);
const Color Color::Black   = Color(0x000000, Color::Name::BLACK,   Color::Group::NEUTRAL);

which can then be used like this :

void fun(const Color& color) {
    switch (color) {
        case Color::Name::RED   : std::cout << "red"; break;
        case Color::Name::GREEN : std::cout << "green"; break;
        case Color::Name::BLUE  : std::cout << "blue"; break;
        // etc.
    }
    std::cout << " ";
    switch (color.group) {
        case Color::Group::PRIMARY   : std::cout << "primary"; break;
        case Color::Group::SECONDARY : std::cout << "secondary"; break;
        case Color::Group::NEUTRAL   : std::cout << "neutral"; break;
    }
    std::cout << " : " << std::hex << color.rgb << std::endl;
}

Upvotes: 1

Mike Seymour
Mike Seymour

Reputation: 254471

I'd use a switch statement.

switch (colour) {
    case Colour::Red:   return Group::Primary;
    //...
    case Colour::Black: return Group::Neutral;
}
return Group::Invalid;  // or throw, assert, or whatever.

That should cover all your needs:

Adds this ugly COUNT item in Color enum (bothers me most)

No need for that, just a case for each enumerator.

Will fail silently if someone reorders items of Color

Each case is explicitly named, so the value of each enumerator doesn't matter (as long as they're unique; but you'll get an error if that's not the case).

Not applicable for not-continuous enums

Again, named case statements don't care about the actual values.

if anyone changes the number of elements in Color enum, this function will fail to compile

While not guaranteed, most compilers should be able to warn if there are unhandled enumerators in a switch (as long as there's no default branch). For GCC, the option is -Wswitch (included in -Wall), along with -Werror if you want it to cause failure.

Upvotes: 3

Related Questions