CODEWITHSUNDEEP
c#if-statement
mowwwalker
mowwwalker

Reputation: 17334

Testing variable against multiple values?

I remember seeing a question on here about the same problem, where things like:

if( x==null || x==" " || x==" " || x=="\n")...

End up becoming long and ugly strings, but the answer wasn't great and I can't remember what it was.

I'm in the middle of a tutorial on MySQL, and the way the problem is solved in SQL is by using the keyword "IN". 

WHERE value IN (1 , 22, 35)...

So I was wondering if it's considered inefficient or bad practice to do:

object[] BadVals = {null, " ", "  ", "\n"};
if(Array.IndexOf(BadVals, x) != -1)...

Upvotes: 4

Views: 4070

Answers (3)

D Stanley
D Stanley

Reputation: 152521

Not necessarily bad practice, but there are some other ways. A couple of different ways are:

  1. For null/empty Strings,

    String.IsNullOrEmpty(s.Trim());

  2. IsNullOrWhiteSpace in 4.0:

     String.IsNullOrWhitespace(s.Trim());

  3. for an operator like SQL IN:

    if((new[] {" ", "  ", "\n"}).Contains(s))
{
}

Upvotes: 3

drf
drf

Reputation: 8699

These matters of style are seldom absolute, and largely dependent on the specific problem that is being solved, as well as organizational considerations and personal preferences. This said, one well-regarded reference (Code Complete by Steve McDonnell) advises to simplify complicated tests with boolean function calls as a means to reduce the complexity in if statements. For instance, one might replace the if statement above with something like:

if (IsStringFoo(s)) {...}

Elsewhere defined as

// Returns true if and only if the input string is a Foo
bool IsStringFoo(string s)
{
    return s == "" || s == " " || s == "\n";
    // or use the alternate array syntax here instead if you prefer
}

This communicates to the consumer not just the logical mechanics, but also potentially the business rationale for these mechanics, thus likely improving readability.

Using the in syntax proposed could suffer from similar readability issues -- in particular, it's still unclear as to why those particular tests are being done. The latter method may be potentially more difficult to understand since the "or" communicates a boolean logic operation in a way Array.IndexOf does not necessarily.

Upvotes: 1

Jon
Jon

Reputation: 437336

It's certainly not efficient in theory as the straight if test, but this is a red herring. The real question is: do you care?

There's two sides to this question.

  1. So what if it's slower? If you aren't running this on a CPU-bound loop that executes a million times, the difference is purely theoretical. Use whatever makes for more bug-free code that's a pleasure to read and maintain.
  2. So what if it's uglier? Are you going to be writing this lots of times? Surely not -- if you intend to use it lots of times, put it inside a well-named method and never think about it again.

As for the LINQ approach, that's slightly shorter and a bit more readable than what you have:

if((new[] { null, " ", "  ", "\n" }).Contains(x)) ...

You 'd probably want to write another extension method that allows you to call this with the operand positions reversed, e.g.

if(x.In(new[] { null, " ", "  ", "\n" })) ...

Verdict: I 'd go with LINQ for more than 3 or so values to test against, provided that there's no other more obvious way to check for these values (e.g. in this case IsNullOrWhiteSpace is awfully close) and there are no obvious performance implications. Otherwise, if is tried and true.

Upvotes: 6

Related Questions