Is this right way to set bits of enum?

Question

I have bitwise enum for save a two state settings of some class. I try to develope easy function to switch the bit ON or OFF. Is this proper and quick way to do it?

enum myEnum { eeNull = 0, ee1 = 1, ee2 = 2, ee4 = 4, ee8 = 8 };

template<class T>
inline T &SetBit(T &eValue, const int iBit, const bool bState)
{
    return eValue = (T) ((eValue| (iBit * bState)) & ~(iBit * !bState));
}

int main()
{
    myEnum eE;

    eE = eeNull;

    SetBit<myEnum>(eE, ee2, true);
    SetBit<myEnum>(eE, ee1, true);
    SetBit<myEnum>(eE, ee2, false);
    return 0;
}

Answer 1

This is how you would do it typically:

enum myEnum {
    ee1 = 1 << 0,
    ee2 = 1 << 1,
    ee3 = 1 << 2,
    ee4 = 1 << 3
};

int foo(int flag) {
    // Turn on ee1
    flag |= ee1;

    // Turn off ee3
    flag &= ~ee3;

    return flag;
}

Answer 2

Your code works but using std::bitset makes it much cleaner.

#include <bitset>
#include <iostream>
enum myEnum { ee1 = 0, ee2 = 1, ee4 = 2, ee8 = 3 };
int main()
{
    std::bitset<4> bE{0};
    bE[ee2] = true;
    bE[ee1] = true;
    bE[ee2] = false;
    std::cout << bE.to_string() << "\n";
}

I bet every new person looking at your code will have to take some time to figure out whether the function actually does what it is supposed to do and why it is written this way.

Edit: I rephrased my answer because of the commotion in the comments.

Answer 3

The usual solution is to overload operators | and & (and |= and &=, of course), using the appropriate casts in the operators, something like:

//  Note that when working with bit masks,
//  it's usual to specify values in hex (or octal or binary).
enum MyEnum {
    eeNull = 0,
    ee1 = 0x01,
    ee2 = 0x02,
    ee4 = 0x04,
    ee8 = 0x08
};

MyEnum
operator|( MyEnum left, MyEnum right )
{
    return static_cast<MyEnum>( static_cast<unsigned>( left ) | static_cast<unsigned>( right ) );
}

MyEnum
operator&( MyEnum left, MyEnum right )
{
    return static_cast<MyEnum>( static_cast<unsigned>( left ) & static_cast<unsigned>( right ) );
}

MyEnum&
operator|=( MyEnum& left, MyEnum right )
{
    left = left | right;
    return left;
}

MyEnum&
operator&=( MyEnum& left, MyEnum right )
{
    left = left & right;
    return left;
}

This allows using the usual operators to set bits. An alternative is to use + to add a value to the set, and - to remove it. This is arguably abuse of operator overloading, since the operations aren't really related to addition and subtraction, but I've found it occasionally useful, and plebiscited by my colleagues.It allows writing: flag -= ee2;, rather than flag &= ~ee2; to turn a bit off.

Answer 4

Yes, your code is correct. It's usually implemented by bitwise & instead of multiplication, but I hope the compiler will understand multiplication by bool too, and produce good code.

Are you sure you need to return a reference to the value? This does no harm directly, but it allows writing complicated expressions with SetBit, which could be dangerous. If you return void instead, it will be harder to write code with side effects.

Bit-fiddling code usually uses unsigned types, because they have defined overflow behavior. Your code uses signed int, which is fine, but for highest bit position (31) may behave in surprising ways (undefined behavior).