views:

188

answers:

5

Usually when I want to check if one condition OR another condition is true, I would write code as follows:

if (i == 5 || j == 3) { // Do whatever here. }

Is there a neat/performant away to avoid the coding style trap of using the OR operator to check for different conditions?

+11  A: 

No. You want to find out if one statement OR the other is true...therefore you use the OR operator. That's not a trap...that's how logic works.

There's really no reason to worry about performance though. Two checks isn't going to hurt anything. Also, in languages that support short-circuit operators, the statement will complete execution after the first expression that evaluates to true.

Justin Niessner
Or you could have `n` nested `if` but there's really no point for that.
Ben
+4  A: 

Since you mention "performant", I'm assuming that you might not be aware that || is short-circuiting, which means that it stops as soon as any part of it is true, and so it won't evaluate more than it has to.

See the documentation for more information.

ho1
+1  A: 

The || operator is efficient because it is an OrElse which means if the first condition is true then the second condition won't be evaluated, it will only be evaluted if the first one is false. The && (AndAlso) operator is the same in that if the second condition will only be evaluated if the first is true.

Paulie Waulie
-1 because, while your answer isn't technically wrong, not everyone is (or needs to be) familiar with VB.NET terminology, especially so with a question tagged `C#`. You should've mentioned "short-circuit operator/evaluation" instead of `AndAlso`/`OrElse`, IMHO.
stakx
@stakx I don't understand your point whatsoever. Yes it was tagged C# but I was using the VB terminology because of the nature of the language in the sense that it is closer to english than C# operators. I realise I didn't point out short-circuiting but I don't think that warrants a downvote. Harsh to say the least, especially as you've gained over 7K points and I have a mere 100.
Paulie Waulie
_@Paulie:_ OK, probably downvoting was a little harsh; but not for the reason you give (me having more points than you), but because your answer was never technically incorrect. (I'm willing to cancel my downvote, but unfortunately I can only do so when the answer is edited.) **Still:** I think it important that your answer mention that `OrElse` and `AndAlso` are *specific* VB.NET terminology, and *not* from Boolean logic *in general*. Without mentioning VB.NET, that's not apparent at all!
stakx
stakx: While `OrElse` may be a VB.Net keyword, it is also the standard .Net terminology, as embodied by the `System.Activities.Expressions.OrElse` class, `System.Linq.Expressions.ExpressionType.OrElse` enumeration member, and `System.Linq.Expressions.Expression.OrElse` member.
Gabe
_@Gabe_, that's true. Not so surprising, either, since LINQ, being a language feature, was likely designed by the same group of people that occupy themselves with the major .NET languages, too. VB.NET might indeed have been the "role model" here. And again (_@Paulie_), I'm willing to cancel my downvote, but due to technical reasons (not stubornness!), I cannot do so *unless* the question is edited *in any way*. I admit I may have simply come at this answer from the wrong angle when I first read it; sorry if this caused bad feelings!
stakx
@stakx To be honest I don't want it to look like I'm winging my way out of a downvote because I think that kind of goes against the principle of the site. In fairness whilst I still believe your vote was harsh I do have to admit that I was a tad defensive. Since joining I have noticed that there is a great community of friendly developers with a knowledge base infinitely greater than my own but also there are a few coding snobs. I think I incorrectly assumed you were the later and so I also apologise :0)
Paulie Waulie
_@Paulie:_ No need for apologising. I in turn will be careful to restrain my voting fury better next time around. ;-)
stakx
@stakx: Last i checked, you could edit the answer (say, to fix a minor grammatical error), and that'd consider the answer "edited" so you could change your vote.
cHao
(_@cHao_: Yes, I've thought about this. However, I personally never feel quite comfortable editing others' answers. And I don't think that this is what the edit facility is there for. If everyone did this just to change a previous vote...)
stakx
I think you are Right Paulie. You rock. That bully with over 1k points robbing you of a point is downright bullying....
brumScouse
A: 

If you really want to avoid use of logical ORs, and if you want to detect if a single variable is one of several possible values (which you're not doing in your example - you've got two), then you could do something like this:

class Program
{
    static void Main(string[] args)
    {
        if (2.ContainedIn(1, 2, 3))
        {
            Console.WriteLine("found it!");
        }
    }
}

public static class ExtensionMethods
{
    public static bool ContainedIn(this int val, params int[] vals)
    {
        return vals.Contains(val);
    }
}

I'm not sure how I feel about this; I don't think it saves much, but might be helpful.

Michael Petrotta
A: 

By coding style trap, do you mean (and we've all seen it) that you just end up adding to the if statement? So you'd eventually get:

if ( a == 2 || a == 3 || a == 5 || a == 14 ) // etc.

as more conditions are added, supported. For example, imagine that a is a message type or tradable commodity and as more message types or commodities are supported, the code gets more and more convoluted.

Additionally, this check can get duplicated around all over the place as programmers who don't understand the original necessarily or are not given the time to refactor simply seek to duplicate this condition without understanding it.

Before you know it, the code is (a) very difficult to read and understand (what does i == 5 || j == 3 actually mean?) and, (b) because you're adding conditions to code, it becomes far more difficult to test.

Much better if possible to put this logic in one place where it can be documented:

inline bool isEngineStarted() { return i == 5 || j == 3; }

if ( isEngineStarted() ) // etc.,

That would be my first consideration - make it work in an understandable and easily maintainable way.

In terms of performance one suggestion you might want to look at is if you have a few different values in a relatively small range to compare to, you might find that a sparse vector of booleans may work better as, in theory, the lookup costs the same regardless of the number of alternative value (i.e. O(1) rather than O(N) ) So if you had, say:

for ( int a = 0; a < 1000000000; ++a )
{
    int c = a % 16;
    if ( c == 2 || c == 3 || c == 5 || c == 14 ) b++;
}

You might find it works quicker like this:

int b = 0;
bool sp[] = { false, false, true, true, false, true, false, false, false, false, false, false, false, false, true, false };

for ( int a = 0; a < 1000000000; ++a )
{
    int c = a % 16;
    if ( sp[ c ] ) b++;
}

I have to say, when I tried this, surprisingly, the first one worked faster! I'd be interested to know if anyone can suggest why.

Robin Welch
Helps if you notice the language! I answered this from a C++ perspective. Hopefully, the concepts are equally transferable.
Robin Welch