Null, Empty Strings and Performance Programmers

Jeff Atwood makes a complaint about “performance programmers” breaking his code.

In his example, Jeff shows the following snippet of code:

If Value <> “” Then
  If nvc.Item(name) = “” Then
    nvc.Add(name, Value)
  End If
End If

… which was then changed to by the performance programmer.:

If Value <> String.Empty Then
  If nvc.Item(name).Equals(String.Empty) Then
    nvc.Add(name, Value)
  End If
End If

The new code now breaks because if the NameValueCollection (nvc) does not have an item in the container with name it’ll return null/nothing, which causes the call to Equals
to fail with a null reference exception. Jeff’s code works because null
references can be compared with empty string in C# and VB.NET.

Comparing null references to empty string is not good programming
practice in general (in C++ such a comparison would cause an
exception). Jeff wrote his code knowing that this comparison was safe
because he knew about the language fundamentals in which he was
developing, the performance programmer did not. This is a good example
of the typical traps that most performance crack teams fall into when
tuning an existing application. Developing software is an art form –
writing good maintainable code that works and performs well sometimes
requires the developer to use non-typical syntax, which can throw other
unfamiliar developers of the code into a loop.

3 thoughts on “Null, Empty Strings and Performance Programmers

  1. Chris Hynes

    I agree that prematurely or overly emphasizing performance is a bad thing, but in this case, I’d argue that Jeff wrote bad code. The code should look like this, for both readability and performance reasons:
    <br>If Value.Length &gt; 0 Then
    <br> If nvc.Item(name) Is Nothing Then
    <br> nvc.Add(name, Value)

  2. Chris Hynes

    Grr… Why does the enter key have to submit a form when you’re trying to write code… Anyway…
    <br>It’s as true now with .NET as it was in the early days of VB: to check for a zero length string ALWAYS ALWAYS ALWAYS use the length of the string rather than comparing to &quot;&quot;. When you compare to &quot;&quot;, not only is it a string comparison (slower), rather than an integer comparison (faster), but a new string is also created.
    <br>Furthermore, the if statement to check if the item is already in the collection may be able to be eliminated entirely. If the idea is to set the value of the item in the collection, regardless of whether it exists or not, just setting the Item will suffice:
    <br>If Value.Length &gt; 0 Then
    <br>&amp;nbsp;&amp;nbsp;nvc.Item(name) = Value
    <br>End If
    <br>The point is, Jeff’s code is unclear because it makes assumptions by not explicitly checking for nulls, and is inefficient because it uses string comparisons where integer comparisions would suffice. A more performant and efficient implementation, in this case at least, is much more simple, clear, and readable.

  3. Jeff Atwood

    I agree with your &quot;he should have used Is Nothing&quot; comment. My use of &lt;&gt; &quot;&quot; doesn’t buy me anything and isn’t as clear as it should be. This is really trivial code in a really trivial part of the app, though, so that wasn’t really my point.
    <br>Beyond that, I think you’re doing exactly what you said you didn’t want to do: prematurely optimizing. And that was my point!

Comments are closed.