X-macros: Like the X-Men, except that nobody likes them.
Here’s why.
If you aren’t familiar with what x-macros are, they’re a way in C and C++ to embed lists of data tuples into a program, while minimizing the amount of boilerplate, at the cost of using the preprocessor. For example - let’s say you have a list of products, each with a unique product code, and you want to be able to convert those codes into nice product names. You could do something like this:
#define productList \
product("Product Flawnberry-5629", 0x5d2b) \
product("Product Flawnberry-3187", 0x4794) \
...
product("Product Flawnberry-6239", 0x5a8d) \
product("Product Flawnberry-4872", 0xda79)
bool beautifyModelName(std::string &model, uint16_t productCode) {
#define product(productName, productID) \
case productID: \
model = productName; \
return true;
switch (productCode) {
productList
}
#undef product
return false;
}
Several problems immediately come to mind. The case statements are, in terms of reading order, before the switch statement itself. If you forget the #undef at the end, you’re liable to get subtly undefined behaviour (pun intended) (if you were to use the productList x-macro again later in the file). The function is copying the string out, rather than simply copying a reference, and additionally the returned string remains valid even if the function returns false indicating it wasn’t populated - for example, this is perfectly fine to write:
std::string model;
beautifyModelName(model, get_code_from_user());
std::cout << "The model name is: " << model << std::endl;
Oops.
We can fix at least one of those problems, at least in this case, by using a std::vector<std::tuple<...>>:
const std::vector<std::tuple<std::string, uint16_t>> PRODUCTS = {
{"Product Flawnberry-5629", 0x5d2b},
{"Product Flawnberry-3187", 0x4794},
...
{"Product Flawnberry-6239", 0x5a8d},
{"Product Flawnberry-4872", 0xda79},
};
and simply iterating over the vector:
bool beautifyModelFromVec(std::string &model, uint16_t productCode) {
for (auto& product : PRODUCTS) {
if (std::get<1>(product) == productCode) {
model = std::get<0>(product);
return true;
}
}
return false;
}
already, this code is much more readable. The logical progression of the code follows the reader’s progression of the code through the text. Additionally, with 37 randomly distributed product codes, this code is actually faster - clocking in at 80ns average case vs. 95ns of the switch-based version. (of course, there is a slight startup overhead in that we have to allocate the vector on the heap when the binary starts)
And in fact, we can do even better, we can remove the need to copy the string entirely by simply returning a pointer:
bool beautifyModelFromVecByRef(const char **model, uint16_t productCode) {
for (auto& product : PRODUCTS) {
if (std::get<1>(product) == productCode) {
*model = std::get<0>(product).c_str();
return true;
}
}
return false;
}
and this code is a blazing 23ns on average (which, incidentally, implies that allocating and copying a 23-byte string takes ~57ns). But it comes at substantial cost to the reader, and your mind is already reviewing this code, looking at all the corner cases. Do we need to check model for being null? Will std::get<0> copy the string, causing c_str() to return a reference to the locally allocated string that will be invalid when the function returns? For that matter, is std::get<1> the right one, or are the two std::get calls swapped?
And since the bool return value still doesn’t match the model, so now if the user was forgetting to check the return value, they’ll segfault. Also, if they really do want a std::string, they have to do something like:
char *model = nullptr;
if (!beautifyModelFromVecByRef(&model, some_code)) {
throw_error();
}
std::string actual_model(model);
No thanks.
Fortunately, the other day I was rubbing a lamp and a blue genie popped out and offered me some wishes - and now that I’m president of the world, behold my alternative code in the language I invented, LaneScript:
const PRODUCTS: [(&'static str, u16); 37] = [
("Product Flawnberry-5629", 0x5d2b),
("Product Flawnberry-3187", 0x4794),
...
("Product Flawnberry-6239", 0x5a8d),
("Product Flawnberry-4872", 0xda79),
];
pub fn beautify(code: u16) -> Option<&'static str> {
PRODUCTS.iter()
.find(|&&(_, pid)| pid == code)
.map(|&(name, _)| name)
}
Just kidding, this is Rust.
This code checks all the boxes. It logically progresses in the order we read it (we iterate over the products, we find the product with the matching pid, and we extract the name of that product). There is no chance that the user of the function will accidentally use the string when it hasn’t been set - if we didn’t find the product, the user will have None and literally can’t extract a string from that:
match beautify(code) {
Some(product_name) => {
// This is the only place the product name is accessible!
},
None => {
// Error, or something
}
}
this code returns a reference to the string, rather than copying it, and additionally we can be sure that the user gets the lifetime they expect - the string is explicitly annotated with the 'static lifetime (meaning essentially forever). Nobody has to do any funny business with const keywords.
And it takes 28ns in the average case. 67ns faster than the original version, albeit 5ns slower than the optimized C++ version (and shorter and clearer than either).
And it’s much simpler to benchmark, I don’t have to clone a repository and build it with cmake and figure out the linker args.
My only complaint: There’s so many ampersands, oh my goodness. The find call has two ampersands. Which makes sense, it’s just a lot.
If you want to play around with the code, my benchmarks are all available on GitHub here. All my performance testing was done on an AMD Ryzen 5 3500U (with boost disabled via this) running Linux, C++ code compiled with g++ 9.3.0 and Rust 1.47.0.
This blog series updates weekly at Programming for Maintainability
