10

We have a property whose job is to look up a description. If the lookup fails it should show an empty string.

So we can code the property like this:

If foo.bar Is Not Nothing Then
  Return foo.bar.Description
Else
  Return String.Empty
End If

But that involves executing foo.bar twice, and if doing so is expensive, it's probably better like this:

Dim b As bar = foo.bar
If b IsNot Nothing Then
  Return b.Description
Else
  Return String.Empty
End If

But really all we want to do is treat any kind of error as an empty description. So in some ways this is simpler:

Try
  Return foo.bar.Description
Catch e As NullReferenceException
  Return String.Empty
End Try

But are there any problems (performance, purity, other?) with just catching and ignoring the error?

You sometimes read it's expensive to throw an exception but I am not sure whether the author means it's expensive to build in exceptions using the Throw keyword (which I am not doing) or whether he means it's expensive to allow exceptions to occur (as I would be doing).

Victor Zakharov
  • 25,801
  • 18
  • 85
  • 151
hawbsl
  • 15,313
  • 25
  • 73
  • 114

3 Answers3

9

If would definitely test for Nothing instead of relying on exceptions here. You code indicates that the scenario where foo.bar is Nothing is an expected scenario, and not an exceptional one. That sort of gives the answer.

Throwing an exception is a relatively expensive operation (from a performance perspective). This is the case regardless of wheter you are throwing it in your code, or if it is thrown in library code; it's exactly the same operation. However, I would not keep from throwing exceptions for performance reasons unless I had a real, measured, critical business case.

In my opinion this is mostly a matter of showing intent; by testing for Nothing and acting gracefully on that, your code expresses the fact that this is not a strange thing to happen.

If you are worried about the performance in executing foo.bar twice, the first thing to do is to find out if that is really the case. If so, there are probably ways of solving that (your code sample already contains a suggestion).

Fredrik Mörk
  • 155,851
  • 29
  • 291
  • 343
4

I would go with this approach:

Dim b As bar = foo.bar
If b IsNot Nothing Then
  Return b.Description
Else
  Return String.Empty
End If

If later find yourself repeating the same portion of the code often, you can wrap this lookup into a generic function, such as this:

Private Function GetPropertyOrStringEmptyIfNothing(barObj As bar, propSelector As Func(Of bar, String))
  If barObj IsNot Nothing Then
    Return propSelector(barObj)
  Else
    Return String.Empty
  End If
End Function

Here is a usage example:

GetPropertyOrStringEmptyIfNothing(foo.bar, Function(x) x.Description)

It will work, assuming you have a similar class declared in the current scope:

Class bar
  Public Description As String
End Class

As far as the cost of catching exceptions, here is a link to my answer on a different question. It gives you an idea of how catching exception impacts debugging performance and that of your Release build. Semantically, you better avoid throwing exceptions if it's non-exceptional situation, as already mentioned in other answers.

Community
  • 1
  • 1
Victor Zakharov
  • 25,801
  • 18
  • 85
  • 151
2

You should always try testing for nothing when you are expecting it as a controlled condition, only use catches where possible to handle unwanted errors (I use unwanted broadly as some errors produce wanted results). The ability to handle null strings without catching an exception is there so use it.

Use an empty string test function within the String class type called IsNullOrEmpty or IsNullOrWhiteSpace:

    Public Shared Sub Main()

        Dim f As String
        Dim b As String

        Dim emptyResponseString As String = "I was empty"

        'Foo will return a Null String'

        f = foo()

        'Bar will return an instantiated String'

        b = bar()

        If String.IsNullOrEmpty(f) Then
            Console.Out.WriteLine("foo(): " & emptyResponseString)
        Else
            Console.Out.WriteLine("foo(): " & f)
        End If

        If String.IsNullOrEmpty(b) Then
            Console.Out.WriteLine("bar(): " & emptyResponseString)
        Else
            Console.Out.WriteLine("bar(): " & b)
        End If

    End Sub

    Public Shared Function foo() As String
        Return Nothing
    End Function

    Public Shared Function bar() As String
        Return "I am not empty!"
    End Function

This will allow you to leave your Exception handling for unexpected exceptions (as it should be ^^ )

In perspective of your choices, the second one is closest to what I would recommend, put your functions result into a holding variable, then test this variable using String.IsNullOrEmpty or if you want to include whitespace checking then String.IsNullOrWhiteSpace (this also tests for empty strings too).

Here is the code running:

http://ideone.com/CelDe

Tom 'Blue' Piddock
  • 2,131
  • 1
  • 21
  • 36