I want to map values of a enum to some other values. For instance, here I map Color
to Group
:
enum class Color {
Red, Green, Blue, Cyan, Magenta, Yellow, White, Black
};
enum class Group {
Primary, Secondary, Neutral
};
One particular interesting case is to map enum to a string (serialize), but I'm interested in the general case. I came out with three possibilities of doing this:
Group GetColorGroupMap(Color color)
{
static const std::map<Color, Group> color2group = {
{ Color::Red, Group::Primary },
{ Color::Green, Group::Primary },
{ Color::Blue, Group::Primary },
{ Color::Cyan, Group::Secondary },
{ Color::Magenta, Group::Secondary },
{ Color::Yellow, Group::Secondary },
{ Color::White, Group::Neutral },
{ Color::Black, Group::Neutral }
};
// Shall I check the iterator here?
return color2group.find(color)->second;
}
Group GetColorGroupArr(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
};
// Shall I check the index here?
return color2group[size_t(color)];
}
Group GetColorGroupSwitch(Color color)
{
switch (color)
{
case Color::Red: return Group::Primary;
case Color::Green: return Group::Primary;
case Color::Blue: return Group::Primary;
case Color::Cyan: return Group::Secondary;
case Color::Magenta: return Group::Secondary;
case Color::Yellow: return Group::Secondary;
case Color::White: return Group::Neutral;
case Color::Black: return Group::Neutral;
}
// Shall I handle this branch here?
throw std::logic_error("How did we get here?");
}
Questions
- What is the best solution among these and why? Maybe there are others?
- Shall I do the checks, mentioned in each function?
- Is there a way to make these functions robust to refactoring, e.g. when another color is added?
My thoughts
About question 1
GetColorGroupMap
: potentially the most slow function, since it traversesstd::map
on each call.GetColorGroupArr
: the least obvious structure, since colors are not mentioned explicitly, they are deduced from array indices. Additionally, it won't work with non-sequential enums.GetColorGroupSwitch
: a little verbose and smells a little old-school. I do not see any particular problems with it here, though.
About question 2
I think no checks are necessary, since failures are possible only if passed Color
is out of specified range, which means, something went wrong before.
About question 3
The only possibility I see is to modify GetColorGroupArr
by adding static_assert
to check that array size is equal to the number of items in Color
. For this, we may add an extra item COUNT
to Color
. I do not like this approach, since we need to check against this special item in every place enum is used.
2 Answers 2
Each of these three are obviously valid, but they also have their problems.
Possibility 1:
This, in my opinion, is the best option as it explicitly matches a color with a group. I have no experience with maps in C++, so I don't know how slow it is, but for this small of a data set, I do not think this will be an issue.
Possibility 2:
I do not recommend this option. It does not show any correlation between the two groups, and it will be easy to break by accidentally adding any updates to the wrong location.
Possibility 3:
This is also a valid option, but I would only use it if the color can be used independent of the group and you need to pair/convert them only in certain uses. Also, this code can be cleaned up a little:
switch (color)
{
case Color::Red:
case Color::Green:
case Color::Blue:
return Group::Primary;
case Color::Cyan:
case Color::Magenta:
case Color::Yellow:
return Group::Secondary;
case Color::White:
case Color::Black:
return Group::Neutral;
}
Edit:
Oops, I missed the three questions.
- I think the top item is the best.
- It would not be a bad idea to do the tests, especially if this code will eventually be maintained by others. However, if it is one-time code that will never be touched after it is tested, it may not be necessary.
- I do not know of any way to make the tests more robust. However, I think the first and third are the least likely to be messed up when updating because it is immediately obvious what you are returning.
-
\$\begingroup\$ Thank you. Any thoughts on question 3? \$\endgroup\$Mikhail– Mikhail2015年02月18日 15:52:57 +00:00Commented Feb 18, 2015 at 15:52
-
\$\begingroup\$ Regarding the speed of maps,
std::unordered_map
(available since C++11) is the equivalent of a hash map in every other language. \$\endgroup\$Qaz– Qaz2015年02月18日 17:33:20 +00:00Commented Feb 18, 2015 at 17:33 -
\$\begingroup\$ @Mikhail Updated. \$\endgroup\$user34073– user340732015年02月18日 20:27:38 +00:00Commented Feb 18, 2015 at 20:27
As requested, the templated approach with additional run-tie dispatch:
template <int Color>
struct Traits
{};
template<>
struct Traits<static_cast<int>(Color::Black)>
{
static const Group group = Group::Neutral;
static std::string asString() { return "black"; }
};
template<>
struct Traits<static_cast<int>(Color::Red)>
{
static const Group group = Group::Primary;
static std::string asString() { return "red"; }
};
template<>
struct Traits<static_cast<int>(Color::Green)>
{
static const Group group = Group::Primary;
static std::string asString() { return "green"; }
};
template<>
struct Traits<static_cast<int>(Color::Magenta)>
{
static const Group group = Group::Secondary;
static std::string asString() { return "magenta"; }
};
Group groupFromColour(Color col)
{
switch (col)
{
case Color::Black:
return Traits<static_cast<int>(Color::Black)>::group;
break;
case Color::Red:
return Traits<static_cast<int>(Color::Red)>::group;
break;
case Color::Green:
return Traits<static_cast<int>(Color::Green)>::group;
break;
// etc.
}
// whatever error handler as before.
}
Note how in the switch
statement, the Black
entry only refers to the Black
Traits
. You might also use a separate Traits
for each mapping, so as you add a need for a new one, existing code is not changed.
traits<>
class defining the mappings at compile time? \$\endgroup\$Color
is extended, not group. \$\endgroup\$