Reputation:
I found a piece of code on the internet that has a really simple objective, yet it uses an ugly approach. Supposedly, the author is using a switch case to determine if a few (non-contiguous) values of a previously-defined Enum belong in their scope. If it does, the function returns true
and that's that. Else, it returns false
.
It practically looks like this:
switch(value) {
case ONE:
case TWO:
/* many similar lines later */
case TWENTY:
case TWENTY_FIVE:
/* an afternoon later */
case ONE_HUNDRED:
return true;
default:
return false;
}
Their use of a switch case is justified with having an instant lookup thanks to a jump table generated by the compiler (even though a jump table doesn't necessarily mean instant lookup from what I've gathered). Even so, this generates countless needless lines of code.
I've read about function inlining and using an array of function pointers, but I don't know how to use that in such a specific case.
How do I avoid writing many lines of case X:
with such a simple case (no pun intended) as this?
Upvotes: 2
Views: 1368
Reputation: 263192
Computing a boolean value efficiently based on some bound integral number is a job for bit twiddling:
const unsigned long long ps[2] = {0x28208a20a08a28ac, 0x800228a202088288};
bool is_prime(unsigned x)
{
return (x < 128) && ((ps[x >> 6] >> (x & 63)) & 1);
}
If you look at the binary representation of the numbers stored in the array, a 1 bit signifies a prime number, and a 0 bit signifies a compound number:
2 8 2 0 8 a 2 0 a 0 8 a 2 8 a c
0010 1000 0010 0000 1000 1010 0010 0000 1010 0000 1000 1010 0010 1000 1010 1100
59 47 41 31 23 17 11 5 2
61 53 43 37 29 19 13 7 3
To scale this to more than 128 numbers, simply increase the array size and patch the <
comparison in is_prime
. The constants 6 and 63 stem from the amount of bits in an unsigned long long
.
Upvotes: 8
Reputation: 101
Since you have an odd enum, skipping values, you can invert your solution. But you will need to declare skipped numbers (if it is skipping only a few).
if( value > ONE_HUNDRED || value < ONE )
{
return false;
}
else
{
switch(value)
{
case SKIPPED_FIRST:
case SKIPPED_SECOND:
{
return false;
}
break;
default:
{
return true;
}
}
}
Upvotes: 0
Reputation: 1829
Avoid a repeating and simple switch case in C/C++?
Yes, do avoid this.. You should normally work at the highest level of abstraction possible at any given domain/context, whether this is compile-time polymorphism (e.g. using templates), object-oriented programming or, simpler control structures.. First come correctness, code clarity and efficiency and then comes optimisation (and only after proper measurement)
In this particular case you could simply do this:
return (value < 100);
I don't see why a 100+ lines switch statement is better.. even if it's faster (which is just an assumption in the absence of actual measurements) it'll only be slightly faster anyway so does it deserve all this fuss?? No, or at least no in most real-life scenarios.. If the application is that critical, optimising such code is better done in assembly language anyway..
As far as function inlining and arrays of function pointers - I'm not sure I understand what the questions is, but nevertheless, if you don't understand how to use such features for optimisation, don't use them.. let the compiler optimise your code instead.
Upvotes: 2