I totally disagree! But it’s an interesting seed for discussion, as it does illustrate how important it is to think about exactly which factors you should think about when trying to improve code.
A “perfect” optimisation is one where you end up with shorter, more readable code that runs faster in all cases, uses less memory at runtime, and is a smaller binary size. But that’s rarely possible, so you have to pick your battles on a case-by-case basis.
In this case, speed is irrelevant. Although being faster is never a bad thing, it needs to be bottom of the priority list for this bit of code.
Compiled code size could possibly be something we care about here. Lots of people may never need this function, or will maybe call it just once, so if it took up megabytes of space in your binary, that’d be a bit annoying. (Though not a huge deal, really)
Runtime memory footprint size may matter a little bit, but again not a huge factor to worry about.
The only really important factors to think about if you were trying to improve this would be:
- How likely are there to be bugs lurking in it.
- If the list of constants changes in the future, how likely is it that someone will make a mistake in updating the code.
You suggested using a regex or dictionary instead. But if you did that:
- It would probably be slower (depends on the number of items and cache locality but my gut feeling is that you’d need enormous lists for it to go faster than a small list of ‘if’ statements)
- It would use more memory at runtime
- It’d involve an extra step to initialise the table, so that’d be called during initialisation (static?), and you’d have to consider multi-threading in terms of how init and lookup are allowed.
- It couldn’t be made constexpr, and even if the linker could see that the function is never called and could remove it, it’d have to still have to retain the code for initialising the map
- A regex would need to be careful not to match patterns other than the valid ones. That to me smells like a source of non-obvious bugs.
- Unless the list of items is gigantic, either a dictionary or regex would make the binary bigger, because those classes are going to be template specialisations, and maybe pull in all kinds of other library dependencies too.
- If you don’t use a switch to do the enum match, then the compiler can’t warn you about new enums that you might need to add.
The only bad aspect I can see in the long explicit lists is that there’s some repetition, so that adding a new item involves editing in two separate places. But there are a couple of tricks that could be used to avoid that, e.g.
- use an X macro. It’s a macro, which is bad, but this would be the most bulletproof solution, since it’d generate code that retains the switch statement and can catch missing items.
- a constexpr array of mappings, and a simple pair of functions to search it. This is pretty good. Short, clean, fast code. But no switch statement, unfortunately.