Use Enum Type as unordered_map Key Instead of Ints #5702
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The datatype for enum and key has already been the same for these. For some the key type was int and the enum is without type which should have defaulted to int.
This changes the key type from int to uint8_t.
This changes the key type from uint8_t to int8_t.
chrstnbwnkl
reviewed
Nov 12, 2025
| {static_cast<int>(Turn::Type::kSharpLeft), "sharp left"}, | ||
| {static_cast<int>(Turn::Type::kLeft), "left"}, | ||
| {static_cast<int>(Turn::Type::kSlightLeft), "slight left"}}; | ||
| const std::unordered_map<Turn::Type, std::string> |
Member
There was a problem hiding this comment.
There are a few examples here (like Turn::Type) which could even be turned into a constexpr std::array<const char*, <number of values>>. Turn type is an uint8_t but in practice we only allow 8 distinct values because we reserve 3 bits for it in DirectedEdge, so the number of values is static anyway.
Contributor
Author
There was a problem hiding this comment.
In this case it can be a switch, returning a const std::string_view.
const std::string_view Turn::GetTypeString(Turn::Type turn_type) {
switch (turn_type) {
case Turn::Type::kStraight:
return "straight";
case Turn::Type::kSlightRight:
return "slight right";
case Turn::Type::kRight:
return "right";
case Turn::Type::kSharpRight:
return "sharp right";
case Turn::Type::kReverse:
return "reverse";
case Turn::Type::kSharpLeft:
return "sharp left";
case Turn::Type::kLeft:
return "left";
case Turn::Type::kSlightLeft:
return "slight left";
}
}?
Member
There was a problem hiding this comment.
Also possible! We have examples for both the array and switch pattern in the code base, I don't have a strong opinion. Maybe the switch is more idiosyncratic here
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Makes sense to use the enum type as a key without the need to cast to int type.
There are a few more maps using uint8_t as key, but the enum is of type int, not sure this should be changed:
valhalla/valhalla/odin/enhancedtrippath.h
Lines 812 to 819 in 3a21fe7