Recently, I encountered program crashes because of unscoped enum as array index pattern once in the workplace and another in an open source library where I added another item in the enum. Evidently, this anti-pattern is very popular among C++ developers.
Open source code using Enum as Array Index pattern without bound check in its constructor. WinToastTemplateType is an unscoped enum. C++11 scoped enum cannot be converted to integer so it is not used for this pattern.
WinToastTemplate::WinToastTemplate(WinToastTemplateType type) : _type(type) { static constexpr std::size_t TextFieldsCount[] = { 1, 2, 2, 3, 1, 2, 2, 3}; _textFields = std::vector<std::wstring>(TextFieldsCount[type], L""); }
For obvious reasons, I cannot show my work code written by my coworker here. This below code snippet have to substitute for our discussion. This type of reckless code in pursuit of performance is not worth having in my opinion. The crash happened just because I unsuspectedly added a new item in the enum that an auto-generated code used to display its name for logging. And the generic error message is not helpful. It did not show the site of crash or the variable name that causes the crash. Just I have said in my previous blog: Put sleep() to make more money from HFT. And Lock-free vs Block-free: Don’t single-minded tracked focus solely on performance just for the sake of it…. Countless hours have been wasted to tracking down buffer overruns in C++ which is not seen in other languages that do bound checking.
enum Fruit { Apple, Orange, Banana }; const std::string& GetFruitName(Fruit fruit) { static const std::vector<std::string> vec{ "Apple", "Orange" }; return vec[fruit]; }
My proposed fix is to use vector’s at() to access the element and catch out_of_range exception and log the function name.
const std::string& GetFruitName(Fruit fruit) { static const std::vector<std::string> vec{ "Apple", "Orange" }; try { return vec.at(fruit); } catch (const std::out_of_range& e) { Log(__FUNCTION__, e.what()); throw; } }
Let me know what you think. Thanks for reading!
Leave a Reply