CODEWITHSUNDEEP
c#enums
destructo_gold
destructo_gold

Reputation: 319

C# Enum Flags Comparison

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.

Upvotes: 7

Views: 20573

Answers (5)

dan
dan

Reputation: 5784

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.

Upvotes: 4

Ryan Emerle
Ryan Emerle

Reputation: 15811

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.

Upvotes: 13

Keltex
Keltex

Reputation: 26426

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))

Upvotes: 26

Andrey Tagaew
Andrey Tagaew

Reputation: 1593

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

Upvotes: 1

Dan Puzey
Dan Puzey

Reputation: 34200

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

Upvotes: 1

Related Questions