Why checking for null before comparing a value to a constant?

Question

Sometimes, a value must be checked for equality with a constant. In such cases, I've always seen the code like this:

if (!string.IsNullOrEmpty(text))
{
    if (text == "Some text here")¹
    {
        // Do something here.
    }
}

In my case, I would rather write:

if ("Some text here".Equals(text))
{
    // Do something here.
}

After all, if text is null, Equals will return false, which is expected. The inversion of a constant and a variable feels strange, but is still understandable for a beginner, and avoids the NullReferenceException which would be thrown with text.Equals("Some text here").

Am I missing something?

Why all source code I've seen use the syntax from the first example, and never from the second one?

^{¹ In real code, it would rather be a constant or a readonly field. To shorten the examples, I put the strings inline.}

I can imagine it coming from the Java world where == checks on Strings against in-code literals return false (due to object reference checks), but in C# if you're just checking for equality you can do the == since the operator overload handles the null case just fine. Your first example seems completely unnecessary. Just do `if (text == "Some text here")` — Chris Sinclair, May 11 '12 at 13:02
You might also be interested in this question; http://stackoverflow.com/questions/44288/differences-in-string-compare-methods-in-c-sharp — dash, May 11 '12 at 13:26

jason · Accepted Answer · 2012-05-11T13:09:15.757

11

In such cases, I've always seen the code like this:

You're right to think that's odd and unnecessary, because it is. It's a completely superfluous null or empty check. Frankly, I'd admonish code like that in a code review.

if (text == "Some text here") {
    // Do something here.
}

is perfectly fine and that's what I'd use.

Am I missing something?

No, you're not missing anything.

Why all source code I've seen use the syntax from the first example, and never from the second one?

Because you're looking for love in all the wrong places?

edited May 11 '12 at 13:09

answered May 11 '12 at 13:01

jason

236,483
35
423
525

Thanks for the useful information and for making me laugh. – Johan Maes Oct 04 '18 at 12:41

score 5 · Answer 2 · answered May 11 '12 at 13:02

There is nothing wrong with just this.

if (text == "Some text here")
{
    // Do something here.
}

There is no need to check for null/empty, since it won't be equal anyway.

If you wish to use the Equals method, there is a version that isn't sensitive to null values.

if (string.Equals(text, "Some text here"))
{
    // Do something here.
}

score 2 · Answer 3 · answered May 11 '12 at 13:03

Your version is fine so long as you have a literal or something that you know will not be null.

If, on the other hand, you had

if (text1.Equals(text2))

then clearly you would need to defend against text1 being null.

But I would stop using Equals here and use == which removes the need for the null check

if (text1 == text2)

Are you preferring to use Equals because of your Java roots?

score 0 · Answer 4 · answered May 11 '12 at 13:02

0

I would think it is in human nature, when you compare smth, you always compare "your" stuff against someone else. not the other way around, i think same natual thinking went to C# coding as well :)

answered May 11 '12 at 13:02

Bek Raupov

3,782
3
24
42

score 0 · Answer 5 · answered May 11 '12 at 13:02

0

I would just use if (text == "Some text here"). It's clear, concise, and fast. The IsNullOrEmpty check you refer to might be a (most likely useless) micro-optimization.

answered May 11 '12 at 13:02

Tim S.

55,448
7
96
122

Why checking for null before comparing a value to a constant?

5 Answers5