7

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.

3
  • 1
    Is it just me who thinks [Flags] for the operation shown is not correct? I am struggling to think of a real world sample where I would have Flags on mathematical operations shown...
    – Sunny
    Apr 2, 2010 at 15:09
  • I agree Sunny. The flags were supposed to represent states in a calculator. I'm not sure why I did it this way. In retrospect, I wouldn't do it this way now days. Sep 28, 2011 at 6:50
  • Possible duplicate of How to Compare Flags in C#? Oct 26, 2015 at 14:01

5 Answers 5

26

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))
1
  • Unless you find this unreadable, do you need seperate HasAllFlags, because you can use HasFlag like so: if(operation.HasFlag(Operations.add | Operations.eval)).
    – weston
    Nov 14, 2012 at 12:55
12

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.

4

Duplicate:

See How to Compare Flags in C#?

C# 4 introduces the Enum.HasFlags() method.

However, see What is it that makes Enum.HasFlag so slow? if perfomance is an issue.

1

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;
}
1

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;
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.