Improve enum conversions #4397
Improve enum conversions #4397
Conversation
|
|
||
| debug_assert (false, "unknown election behavior"); | ||
| return {}; | ||
| auto value = magic_enum::enum_cast<nano::stat::detail> (magic_enum::enum_name (behavior)); |
There was a problem hiding this comment.
I am not convinced this is an improvement overall, although the code brevity is a nice thing.
If a new behaviour is added that doesn't exist as a detail then the problem will be silently ignored whereas, in the previous case, the compiler would complain that not all values were handled by the switch case statement.
I am not objecting to the change, just wondering what this gives us, other than a few less lines of very obvious code.
There was a problem hiding this comment.
I hope you're joking... This used to be and still is one on the most annoying and unnecessary things to do in this codebase.
| ASSERT_FALSE (nano::to_string (nano::stat::type::_last).empty ()); | ||
| ASSERT_NO_THROW (std::string{ nano::to_string (nano::stat::type::_last) }); | ||
| ASSERT_EQ (nano::to_string (nano::stat::type::_last), "_last"); |
There was a problem hiding this comment.
Doesn't this line do the job of the both lines above it?
There was a problem hiding this comment.
What happens if nano::to_string (nano::stat::type::_last) returns a malformed string view with size set to a random value? This seems to be fixed in the latest magic enum version but was a problem before. If assert_eq fails it's most likely going to print this large, malformed string. Byt here it will get caught sooner. Also, I added those lines so they follow a pattern.
There was a lot of handwritten code doing simple enum conversions, and thanks to magic enum library this is unnecessary. Both
magic_enum::enum_cast (...)andmagic_enum::enum_name (...)are constexpr, so depending on how good the compiler is, the code shouldn't be slower than what we had before.