C# Enum Flags Comparison

Question

Given the following flags,

  [Flags]
    public enum Operations
    {
        add = 1,
        subtract = 2,
        multiply = 4,
        divide = 8,
        eval = 16,
    }

How could I implement an IF condition to perform each operation? In my attempt, the first condition is true for add, eval, which is correct. However the first condition is also true for subtract, eval, which is incorrect.

        public double Evaluate(double input)
    {
        if ((operation & (Operations.add & Operations.eval)) == (Operations.add & Operations.eval))
            currentResult += input;
        else if ((operation & (Operations.subtract & Operations.eval)) == (Operations.subtract & Operations.eval))
            currentResult -= input;
        else
            currentResult = input;

        operation = null;

        return currentResult;
    }

I cannot see what the problem is.

Answer 1

Change your inner & to |:

if ((operation & (Operations.add | Operations.eval)) == (Operations.add | Operations.eval))

This is equivalent to:

if( ((operation & Operations.add)==Operations.add) &&
    ((operation & Operations.eval)==Operations.eval))

which might be more readable. You might also want to consider an Extension like this:

public static bool HasFlag(this Operations op, Operations checkflag)
{
    return (op & checkflag)==checkflag;
}

then you can do this:

if(operation.HasFlag(Operations.add) && Operations.HasFlag(Operations.eval))

which might be even more readable. Finally you could create this extension for even more fun:

public static bool HasAllFlags(this Operations op, params Operations[] checkflags)
{
    foreach(Operations checkflag in checkflags)
    {
        if((op & checkflag)!=checkflag)
            return false;
    }
    return true;
}

Then your expression could turn into:

if(operation.HasAllFlags(Operations.add, Operations.eval))

Answer 2

The reason your operation is failing is because you have the wrong expression. (Operations.add & Operations.eval) is always zero. Left and right side of your first comparison are both always zero. Try this instead - I suspect it's what you were after:

public double Evaluate(double input)
{
    if ((operation & (Operations.add | Operations.eval)) == (Operations.add | Operations.eval))
        currentResult += input;
    else if ((operation & (Operations.subtract | Operations.eval)) == (Operations.subtract | Operations.eval))
        currentResult -= input;
    else
        currentResult = input;

    operation = null;

    return currentResult;
}

Answer 3

Try this:

   public double Evaluate(double input)
{
    if ((operation & (Operations.add | Operations.eval)) == (Operations.add | Operations.eval))
        currentResult += input;
    else if ((operation & (Operations.subtract | Operations.eval)) == (Operations.subtract | Operations.eval))
        currentResult -= input;
    else
        currentResult = input;

    operation = null;

    return currentResult;
}

Answer 4

Wow, I can't believe all of the wrong answers..

It's important to understand bitwise math if you're working with flags. In your case, you have the following (for the first condition):

1 in binary is  00001
16 in binary is 10000

  00001
& 10000
--------
  00000

So, say we have Subtract (2) as the operation

2 in binary is     00010
previous result is 00000

  00010
& 00000
--------
  00000

Since the previous result is 00000 anything AND'd with it will be zero. So your condition will always evaluate to true since 0 == 0.

If we just switch this to OR, then we have the following:

1 in binary is  00001
16 in binary is 10000

  00001
| 10000
--------
  10001 (17)

Now, say we have Add (1) as the operation

1 in binary is     00001
previous result is 10001 (17)

  00001
& 10001
--------
  00001

So, 1 & 17 => 1 and thus your final condition is (1 & ( 1 | 16 ) ) == ( 1 | 16 ) => 1 & 17 == 17 => 1 == 17 => false (still false!)

So what you actually want is:

((operation | Operations.add | Operations.eval) & (Operations.add | Operations.eval)) == (Operations.add | Operations.eval)

This becomes ((1 | 1 | 16 ) & ( 1 | 16 )) == ( 1 | 16 ) => ( 17 & 17 ) == 17 => 17 == 17 == true

This is obviously not readable, so you should opt for extracting this into a method (as suggested). But it's still important to understand why your condition is incorrect.