18

MS Analyzer recommends to use string.IsNullOrEmpty instead of comparising it either with null or empty string for performance reasons

Warning 470 CA1820 : Microsoft.Performance : Replace the call to 'string.operator ==(string, string)' in ... with a call to 'String.IsNullOrEmpty'.

Why is that? Shouldn't the requirement to call another function and pass it reference to some object, which then needs to execute some kind of comparison anyway, be more expensive than executing comparison itself?

Example code

void Foo()
{ // throws a warning
    string x = "hello world";
    if (x == null || x == "")
    {
        Console.WriteLine("Empty");
    }
}

void Foo()
{ // doesn't throw it
    string x = "hello world";
    if (string.IsNullOrEmpty(x))
    {
        Console.WriteLine("Empty");
    }
}
Petr
  • 13,747
  • 20
  • 89
  • 144
  • 2
    Show the code you're comparing `String.IsNullOrEmpty` with. – MarcinJuraszek Aug 29 '13 at 09:56
  • possible answer http://stackoverflow.com/questions/10360370/why-is-string-isnullorempty-faster-than-string-length – Sachin Aug 29 '13 at 09:56
  • http://www.dotnetperls.com/isnullorempty – cnd Aug 29 '13 at 09:57
  • @Sachin the question you link asks why the method performs faster than getting the length of the string and is the result of the user not setting up a proper test environment. This question asks why the method is recommended instead of the more intuitive `(x == null || x == "")`. The questions are not duplicates. – Trisped Jan 26 '16 at 21:05
  • This is the type of silliness that people get into...unless you are using it in some giant loop or calling it routinely, it doesn't matter. Will you be able to tell the 1.5ms difference in speed by calling IsNullOrEmpty versus == "" ?? a few times here and there... NO! – MattE Oct 29 '17 at 15:40
  • 1
    If people get into this kind of "silliness" as you call it, MattE, then there must be a reason. Curiosity, perhaps? I know that's why I looked into it, plus I wanted a deeper understanding of how the two approaches to essentially the same problem differ. The knowledge I gained since researching it has already proven helpful, so the fact that it may be more of a theoretical problem than a practical one is no reason to call people silly, at least not in my book ;) – Thomas Nov 06 '20 at 11:00

2 Answers2

19

MS Analyzer recommends to use string.IsNullOrEmpty instead of comparising it either with null or empty string for performance reasons

Warning 470 CA1820 : Microsoft.Performance : Replace the call to 'string.operator ==(string, string)' in ... with a call to 'String.IsNullOrEmpty'.

Just read the fine manual:

A string is compared to the empty string by using Object.Equals.

...

Comparing strings using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. This is because Equals executes significantly more MSIL instructions than either IsNullOrEmpty or the number of instructions executed to retrieve the Length property value and compare it to zero.

...

To fix a violation of this rule, change the comparison to use the Length property and test for the null string. If targeting .NET Framework 2.0, use the IsNullOrEmpty method.

Your problem is not so much the null check, but instead testing for equality (via Equals) with an empty string instance rather than checking its Length.

Again, from the fine manual:

  public void EqualsTest()
  {
     // Violates rule: TestForEmptyStringsUsingStringLength. 
     if (s1 == "")
     {
        Console.WriteLine("s1 equals empty string.");
     }
  }

  // Use for .NET Framework 1.0 and 1.1. 
  public void LengthTest()
  {
     // Satisfies rule: TestForEmptyStringsUsingStringLength. 
     if (s1 != null && s1.Length == 0)
     {
        Console.WriteLine("s1.Length == 0.");
     }
  }
Community
  • 1
  • 1
ta.speot.is
  • 26,914
  • 8
  • 68
  • 96
2

IsNullOrEmpty will be inlined so the overhead of calling the method will be avoided. Looking at the method, it is decorated with the attribute

[__DynamicallyInvokable, TargetedPatchingOptOut("Performance critical to inline across NGen image boundaries")]

I'd also add that IsNullOrEmpty is clearer and more descriptive from a readability point of view (in my opinion).

As for performance, I'd be surprised if there was any real difference if you were to use value.Length == 0; instead of x == "". Internally, IsNullOrEmpty does this

return value == null || value.Length == 0;

not

if (x == null || x == "")

Reading a property requires less overhead than calculating equality.

keyboardP
  • 68,824
  • 13
  • 156
  • 205
  • 2
    So what performance will be gained by calling String.IsNullOrEmpty(var) instead of (var == null || var == String.Empty)? – sisve Aug 29 '13 at 09:57
  • "clearer and more descriptive?" How much clearer and more descriptive than `str != null && str != ""` can you get? – Jon Aug 29 '13 at 09:59
  • @Jon - Fair enough, I've added IMO. Having said that, English would be clearer than symbols for me. `IsNullOrEmpty` is self explanatory, `str != null && str != ""` is not as much (although not exactly hard to decipher) IMHO. – keyboardP Aug 29 '13 at 10:03
  • 1
    I never said that I did any kind of benchmark. Just ms visual studio code analysis returned several performance warnings telling me that I should change that code (probably it wouldn't recommend it if there was no performance difference) – Petr Aug 29 '13 at 10:03
  • 1
    @Petr - You're code is not performing the same check as `IsNullOrEmpty`. You're doing an equality check whereas `IsNullOrEmpty` just reads the `Length` property. – keyboardP Aug 29 '13 at 10:04
  • @SimonSvensson - Sorry, read your comment wrong the first time. I thought you were comparing it with what `IsNullOrEmpty` uses (it does *not* use `var == String.Empty`). The performance gain is from avoiding an equality check with `String.Empty`. I highly doubt this performance gain is important in the majority of cases but reading the `Length` property requires less overhead than checking equality. – keyboardP Aug 29 '13 at 10:11
  • @Jon - `IsNullOrEmpty` is also less error prone as shown by your comment ;) (should be `str != null || str != ""`) – keyboardP Aug 29 '13 at 10:19
  • @keyboardP: Depends really -- to be frank I had `if (!IsNullOrEmpty)` in mind because that what I 'd be most likely to use in code. With the negation taken into account `&&` is correct, otherwise the correct is `str == null || str == ""` (`==`, not `!=`). Your suggestion is going to be always `true`, which is a bug but I have to admit definitely reinforces the "less error prone" conclusion. ;-) – Jon Aug 29 '13 at 10:43
  • @Jon Haha there we go, it just encourages confusion :D I agree though, more times than not, I use `!String.IsNullOrEmpty` too. You're right that it is subjective so was happy to add IMO at the end. – keyboardP Aug 29 '13 at 10:47