Concat string in IsNullOrEmpty parameter

I was looking at a piece of code I wrote in C#:

if(string.IsNullOrEmpty(param1) && string.IsNullOrEmpty(param2) && string.IsNullOrEmpty(param3))
{
       // do stuff
}

and decided to make it more readable/concise

if(string.IsNullOrEmpty(param1+param2+param3))
{
       // do stuff
}

But looking at it I can't help but cringe. What are your thoughts on this? Have you ever done something like this and do you use it whenever applicable.

Note: The code previous开发者_开发知识库 to this line would manipulate a collection by adding specific items depending on if the a param (param1,param2,param3) is NOT empty. This if statement is meant for validation/error handeling.

Personally I prefer the former over the latter. To me the intent is more explicit -- checking if all parameters are null/empty.

The second also hides the fact it does handle nulls. Null strings are odd. Jason Williams above, for example, didn't relise that it does in fact work.

Maybe write it something like this, which is a bit more readable:

bool paramsAreInvalid =
   string.IsNullOrEmpty(param1)
   && string.IsNullOrEmpty(param2)
   && string.IsNullOrEmpty(param3);

if (paramsAreInvalid)
{
       // do stuff
}

It's a small thing, but I think a minor reformatting of your original results in improved readability and makes the intent of the code about as crystal clear as can be:

if ( string.IsNullOrEmpty(param1) && 
     string.IsNullOrEmpty(param2) && 
     string.IsNullOrEmpty(param3) )
{
       // do stuff
}

Consider this similar set of examples:

if ( c == 's' || c == 'o' || c == 'm' || c == 'e' || c == 't' || c == 'h' || c == 'i' || c == 'n' || c == 'g') {
    // ...
}

if ( c == 's' || 
     c == 'o' || 
     c == 'm' || 
     c == 'e' || 
     c == 't' || 
     c == 'h' || 
     c == 'i' || 
     c == 'n' || 
     c == 'g') {
    // ...
}

That won't work. If any of the strings are null, you'll get a null dereference exception. You need to check them before you use them.

In addition, it's very inefficient. You are concatenating all the strings into a new string, then test if this is non-empty. This results in one or more memory allocations and potentially a lot of data being copied, only to be immediately thrown away and garbage collected a moment later.

A better approach is to write a method that takes variable arguments or a list of strings and checks them one by one using IsNullOrEmpty in a loop. This will be more efficient, safe, but still achieve the desired result of tidy code in your if statement.

If you can get the params in a collection (which if it's function you can with the params keyword) then this might work:

if (myParams.Any(IsNullOrTrimEmpty)
    {
        // do stuff
    }

The example uses this string extension and myParams is a string[].

The original code, though longer, is more clear in its intent, and likely similar performance-wise. I'd leave it alone.