CODEWITHSUNDEEP
c++bit-manipulation
ontherocks
ontherocks

Reputation: 1999

Bit flags as input in a routine for validating

#include <iostream>

using namespace std;

enum Property
{
    HasClaws   = 1 << 0,
    CanFly     = 1 << 1,
    EatsFish   = 1 << 2,
    Endangered = 1 << 3
};

bool isHawk(int prop)  // <-- Is the input type correct?
{
    //  If it can fly then also it is a hawk. If it has claws then also it is a hawk.
    if (prop& HasClaws || prop& CanFly)
    {
        return true;
    }

    //  If it can fly as well has claws then also it is a hawk.
    if ((prop& (HasClaws | CanFly)) == (HasClaws | CanFly))  //<-- Can this be written cleaner/simpler
    {
        return true;
    }

    return false;
}

int main(int argc, char *argv[])
{
    cout << "Value = " << isHawk(CanFly | HasClaws) << endl;

    system("PAUSE");
    return EXIT_SUCCESS;
}

Couple of my questions are inline in the above code.

In the second if condition if ((prop& (HasClaws | CanFly)) == (HasClaws | CanFly)), what I really wanted to check is, if it can both fly as well as has claws. Is OR the right operator for this or should it be AND? Same is the question while calling isHawk(CanFly | HasClaws).

In general, can isHawk() as written above, be written in a simpler/cleaner way?

This is just an example code. It has nothing to do with hawks or being a hawk.

Upvotes: 0

Views: 39

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 597941

Is the input type correct?

Defining prop as int is fine. Just know that you have 28 unused bytes. You might consider using an unsigned char or unsigned short instead to reduce the number of bits used.

Can this be written cleaner/simpler

You could add another value to your enum to combine the HasClaws and CanFly bits under a single name:

enum Property
{
    HasClaws   = 1 << 0,
    CanFly     = 1 << 1,
    EatsFish   = 1 << 2,
    Endangered = 1 << 3,
    HasClawsAndCanFly = HasClaws | CanFly
};


if ((prop & HasClawsAndCanFly) == HasClawsAndCanFly)

In the second if condition if ((prop& (HasClaws | CanFly)) == (HasClaws | CanFly)), what I really wanted to check is, if it can both fly as well as has claws. Is OR the right operator for this or should it be AND?

The | is correct. The real problem is the || in the first if. If you pass in either HasClaws or CanFly by itself, you are returning true when you should be returning false instead:

isHawk(HasClaws) // returns true
isHawk(CanFly) // returns true
isHawk(HasClaws | CanFly) // returns true
isHawk(HasClawsAndCanFly) // returns true

You need to remove the first if altogether:

bool isHawk(int prop)
{    
    if ( (prop & (HasClaws | CanFly)) == (HasClaws | CanFly))
    //if ( (prop & HasClawsAndCanFly) == HasClawsAndCanFly)
    {
        return true;
    }

    return false;
}

Which can then be simplified:

bool isHawk(int prop)
{    
    return ((prop & (HasClaws | CanFly)) == (HasClaws | CanFly));
    //return ((prop & HasClawsAndCanFly) == HasClawsAndCanFly);
}

Upvotes: 1

Related Questions