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.
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))
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With