2

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.

someone
  • 101
  • 2
  • 12
  • 4
    Sounds like XY-problem to me. Why would you want to do this? – Eugene Sh. Jul 19 '18 at 18:10
  • If it needs to scale, use a hashmap to look up. https://stackoverflow.com/questions/838404/implementing-a-hashmap – stark Jul 19 '18 at 18:12
  • 2
    Why do you have an ENUM with random values like that, instead of sequential numbers? If they were sequential you could just check the range. – Barmar Jul 19 '18 at 18:14
  • From your code, it looks to me you want to see whether an enum is actually *outside* any defined values - That looks like a different problem to me. – tofro Jul 19 '18 at 18:35
  • 1
    @Barmar having something other than a sequential range is pretty common in some software technology areas. Embedded comes to mind. – Richard Chambers Jul 19 '18 at 18:38
  • Without providing the _why_, or additional restrictions, there is no better way. VTC as too broad as OP has not responded to prior clarification questions. – chux - Reinstate Monica Jul 19 '18 at 19:27
  • As I mentioned in my EDIT, the numbers aren't sequential and I can't change this fact – someone Jul 19 '18 at 19:43

3 Answers3

4

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;
}
coder3101
  • 3,920
  • 3
  • 24
  • 28
  • Yes, that can work too, OP write int x, so I wrote it same way. No worries I will change it – coder3101 Jul 19 '18 at 18:44
  • 1
    Bonus points if you add alternative names to the first and the last values, something like `Begin` and `End`. – HolyBlackCat Jul 19 '18 at 19:04
  • 1
    Case 2 can be handled a little better. `enum Bar { FIRST=10, MIDDLE, LAST, OUT_OF_RANGE }`. Then use `&& x < OUT_OF_RANGE`. This allows you to add new elements to the enum by putting them before `OUT_OF_RANGE` and you don't have to change the validation function. – Barmar Jul 19 '18 at 19:28
1

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.

Martijn Otto
  • 878
  • 4
  • 21
0

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.
Richard Chambers
  • 16,643
  • 4
  • 81
  • 106