Why does this IF statement return false?

Question

I have two text boxes and I want skip a block of code only when both are empty:

if (txtBox1.Text.Trim() != string.Empty && txtBox2.Text.Trim() != string.Empty )
{
     // Do something
}

If either of the text boxes has something, I want the //Do something part to execute. Only when both are empty do I want to skip.

However the above code fragment doesn't work. Why?

Answer 1

You should replace your && with ||. Currently, the code in the if block will only be executed if both text fields have text in them.

Answer 2

I believe you have your logical operators mixed up. What you're looking for is

if (txtBox1.Text.Trim() != string.Empty || txtBox2.Text.Trim() != string.Empty )
{
     // Do something
}

Answer 3

You want || instead of &&.

Answer 4

if (txtBox1.Text.Trim() != string.Empty || txtBox2.Text.Trim() != string.Empty ) { // Do something }

You are using && which will require BOTH of them do be NOT empty. You should use || (OR) to get the desired result.

Answer 5

Might be easier to understand:

if (!(txtBox1.Text.Trim() == string.Empty && txtBox2.Text.Trim() == string.Empty ))
{
     // Do something
}

Answer 6

Those who pointed out you need || instead of && are right. If you prefer &&, you could also use:

if (!(txtBox1.Text.Trim() == string.Empty && txtBox2.Text.Trim() == string.Empty))
{
     // Do something
}

The difference is purely aesthetic, but amounts to checking (in English): "It isn't true that both textboxes are empty" as opposed to "It's true that at least one textbox isn't empty." Same meaning, different way of putting it.

For the record, the way you had it in your original question was: "It's true that both textboxes are not empty."

Answer 7

I've split the BothBoxesEmpty out to a separate variable to make it more readable... all the brackets become an unreadable mess otherwise. You can merge the two statements if you choose:

bool BothBoxesEmpty = string.IsNullOrEmpty(TextBox1.Text.Trim()) &&
                      string.IsNullOrEmpty(TextBox2.Text.Trim());
if (!BothBoxesEmpty)
{
    /* Do your stuff */
}

You could equally easily replace the string.IsNullOrEmpty with TextBox1.Text.Trim() == string.Empty as TextBox1.Text will always return a string (empty or not) and will never return null.

Answer 8

If you're in a method you could do something like this as well:

 if (txtBox1.Text.Trim() == string.Empty && txtBox2.Text.Trim() == string.Empty )
      return;
 //Do Something

Answer 9

You could make it more concise by concatenating the strings, then testing:

if ((txtBox1.Text.Trim() + txtBox2.Text.Trim()) != string.Empty )
{
    // Do something
}

Depending on what the boxes represent this could be somewhat less obvious, though.

Answer 10

This reminds me of a story I heard about a junior coder who was told to implement rules for calculating insurance benefits, where "everyone under 18 and over 65" did not qualify. So he coded it as:

if (age < 18 and age > 65) ...

The story goes that his coworkers tried repeatedly to convince him that his logic would not work correctly, but he stubbornly refused to understand or change it.