Avoiding the coding trap of using || to check for different conditions [closed]

It's difficult to tell what is being asked here. This question is ambiguous, vague, incomplete, overly broad, or rhetorical and cannot be reasonably answered in its current form. For help clarifying this question so that it can be reopened, visit the help center. Closed 12 years ago.

Usually when I want to check if one co开发者_运维技巧ndition 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?

---

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.

---

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.

---

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.

---

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.

---

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.