Reputation: 101
my question is related to appearance of the code and how is it doing in terms of performance.
let's say i have an enum:
typedef enum {
THIS_IS_A_VALUE1 = 65,
THIS_IS_A_VALUE2 = 10,
THIS_IS_A_VALUE3 = 45,
THIS_IS_A_VALUE4 = 5,
THIS_IS_A_VALUE5 = 7
} I_AM_AN_ENUM;
and i want to check whether a certain variable contains one of the values in the enum: i thought about three ways to do it:
1.
int val;
if(val != THIS_IS_A_VALUE1 && val != THIS_IS_A_VALUE2 && val != THIS_IS_A_VALUE3...)
{
// val is different than all values
// do something
}
2.
if(val != THIS_IS_A_VALUE1)
{
// keep blank
}
else if(val != THIS_IS_A_VALUE2)
{
// keep blank
}
else if(val != THIS_IS_A_VALUE3)
{
// keep blank
}....
else
{
// val is different than all values
// do something
}
3.
switch(val)
{
case THIS_IS_A_VALUE1:
case THIS_IS_A_VALUE2:
case THIS_IS_A_VALUE3:
..
..
default:
// val is different than all values
// do something
}
to be honest, i find 2 and 3 ugly, especially when there are a lot of values in the enum. the first way can also be pretty ugly but better then the others, though using a lot of && can reduce from the performance from my understanding.
so to summarize: readability and performance are very important to me, and i'm really unsure which of the options is the best, or maybe there are other ways to do it?
thanks.
EDIT: i forgot to mention that the values are not sequential, that's the case and I can't really change it.
Upvotes: 2
Views: 3926
Reputation: 4165
An enum is essentially the same thing as using macros to define constants, except that the enum wraps a set of associated constants up into a data type. This makes your code more self-documenting, but doesn't really provide any additional functionality.
Here are two cases that arise to your Question.
Case 1 : Values in Enums are not sequencial, means
enum Foo { FIRST=0, MIDDLE=40, LAST=12 };
bool isInFoo(enum Foo x){
bool found = false;
switch(x){
case FIRST: found = true; break;
case MIDDLE: found = true; break;
case LAST: found = true; break;
}
return found;
}
In this way, if you do not handle all enums inside switch, you will get a compile time warning.
Case 2 : Values are Sequencial. Then to check if enum Bar x
is in enum, you can do something like this
enum Bar { FIRST=10, MIDDLE=11, LAST=12 };
bool isInBar(enum Bar x){ return x >= FIRST && x <= LAST; }
EDIT : As per comments below, you could add a Dummy Last entry to Bar
as OUT_OF_RANGE = LAST+1
and then in the Function isInBar()
you can check for x less than OUT_OF_RANGE, this way if we add more element to our enum, we will not have to change the isInBar() function, because newly added value should be less than OUT_OF_RANGE. In the end it depends on you whether to include or not.
Please note in case 2, the enum values do not necessarily need to be a continuous Sequencial numbers, you can keep them in any series you wish, but then you need to change your isInBar()
accordingly.
enum FooBar { FIRST=3, SECOND=6, THIRD=9, FOURTH=15, LAST=12 };
bool isInFooBar(enum FooBar x){
bool found = false;
for(int t=3;t<=15;t+=3)
if(x == t) {
found = true;
break;
}
return found;
}
Upvotes: 4
Reputation: 17573
You may want to encapsulate the check logic and use special names in order to perform the range check. Taking this approach provides some very nice set logical operations like check if in this set of values or check if it is in this set intersect with another set, etc.
I haven't tested any of this though I did a compile with Visual Studio 2005 in a C++ file and it seems to compile with that. In other words, you may need to make some adjustments to this.
typedef enum {
THIS_IS_A_VALUE1 = 65,
THIS_IS_A_VALUE2 = 10,
THIS_IS_A_VALUE3 = 45,
THIS_IS_A_VALUE4 = 5,
THIS_IS_A_VALUE5 = 7
} I_AM_AN_ENUM;
typedef enum {
IS_A_THIS_IS_A_VALUE1 = 0x01,
IS_A_THIS_IS_A_VALUE2 = 0x02,
IS_A_THIS_IS_A_VALUE3 = 0x04,
IS_A_THIS_IS_A_VALUE4 = 0x08,
IS_A_THIS_IS_A_VALUE5 = 0x10,
IS_A_THIS_IS_VALUE123 = 0x07, // bitwise Or of first three masks.
IS_A_THIS_IS_VALUE_ANY = 0x1f // is any of the values.
} IS_A_I_AM_ENUM;
struct {
I_AM_AN_ENUM eVal;
ULONG ulMask;
} testArray[] = {
{THIS_IS_A_VALUE1, IS_A_THIS_IS_A_VALUE1},
{THIS_IS_A_VALUE2, IS_A_THIS_IS_A_VALUE2},
{THIS_IS_A_VALUE3, IS_A_THIS_IS_A_VALUE3},
{THIS_IS_A_VALUE4, IS_A_THIS_IS_A_VALUE4},
{THIS_IS_A_VALUE5, IS_A_THIS_IS_A_VALUE5}
};
bool isInBar (I_AM_AN_ENUM x, ULONG ulCheckMask)
{
int i;
for (i = 0; i < sizeof(testArray)/sizeof(testArray[0]); i++) {
if (testArray[i].eVal == x) {
if (testArray[i].ulMask & ulCheckMask) return true;
else break;
}
}
return false;
}
// check a value against a range.
I_AM_AN_ENUM eVal = THIS_IS_A_VALUE3;
// check if eVal is Value3 or a Value 4
if (isInBar (eVal, IS_A_THIS_IS_A_VALUE3 | IS_A_THIS_IS_A_VALUE4)) // do something
if (isInBar (eVal, IS_A_THIS_IS_VALUE123 & ~IS_A_THIS_IS_A_VALUE1)) // do something
if (isInBar (eVal, IS_A_THIS_IS_VALUE_ANY)) // do something.
Upvotes: 0
Reputation: 938
I don't think there is a 4th way that would be better. I do think option 3 is the best option because this way the compiler can check whether you have handled all cases.
This is tremendously helpful when you decide to add an option to the enum. The compiler can then give you a warning so you don't forget to include the new case.
Upvotes: 1