3 ms·
This is ridiculous. Sure, I'll agree, it's a neat trick, but what mindset do you need to be in to come up with such a convoluted solution to such a problem, and
by jmts 10y ago
This is ridiculous. Sure, I'll agree, it's a neat trick, but what mindset do you need to be in to come up with such a convoluted solution to such a problem, and the wrong problem at that?
What problem does hack this solve? Single point of edit for a new feature? Well, odds are you've added/edited code elsewhere to implement the feature. Saving one edit is pointless. Just do it properly.
What problem is this code trying to solve? Message dispatch. And it does it. Twice. Once to differentiate whether something is a command or a status message, and then once again to actually execute the thing. Silly. Just build a table and be done with it. If you're concerned about performance after you've benchmarked it, write some code to generate a perfect hash and generate your table.
static const struct message_handler {
int id;
void (*process)(int x);
} tab[] = {
{ .id = CMD1, .process = process_cmd1, },
{ .id = CMD2, .process = process_cmd2, },
{ .id = CMD3, .process = process_cmd3, },
// ...
{ .id = STATUS1, .process = process_status1, },
{ .id = STATUS2, .process = process_status2, },
{ .id = STATUS3, .process = process_status3, },
};
const struct message_handler* find_message_handler(int x);
void process_message(int x) {
const struct message_handler *handler;
handler = find_message_handler(x);
if (NULL != handler) {
handler->process(x);
}
else {
report_error(x);
}
}
Now tell me which one your code reviewers and juniors are going to prefer.
- zik 10y ago> What problem is this code trying to solve? I think you missed the point of this hack - the problem he's trying to solve is described in the article: > The common issue associated with switch statement is typically maintenance; especially where the set of ‘valid’ values needs extending. He wants the efficiency of the switch statement for all the cases he knows about and a fallback to the less efficient if clauses if that fails due to new cases being added. It's a pretty silly example but let's face it - it's more for fun than elegance.
- jmts 10y agoI see your point, but I'd argue that even then it's still the wrong the problem. 1. If you don't enjoy maintaining your switch statement, it's probably huge anyway and you need to refactor. 2. If you don't enjoy maintaining this switch statement, you probably also don't enjoy maintaining the switch statement inside next function that gets called that gives you your full set of valid values. 3. The number of types of messages used is probably much fewer than unique messages, so you are optimising on a small N, which will give minimal returns. 4. You never need to look at this code again and it will just sit there adding overhead. Other switch statements will be maintained because they're the first place you'll notice missing entries. Fun, sure. Like setting a fistful of matches on fire. It takes a bit of effort to set up, and it looks interesting. I'll agree to that.