Exceptional String Comparisons – The Daily WTF

As a general rule, I will actually prefer code that is verbose and clear over code that is concise but makes me think. I really don’t like to think if I don’t have to.

Of course, there’s the class of WTF code that is verbose, unclear and also really bad, which Thomas sends us today:

Private Shared Function Mailid_compare(ByVal queryEmail As String, ByVal FnsEmail As String) As Boolean
    Try
        Dim str1 As String = queryEmail
        Dim str2 As String = FnsEmail
        If String.Compare(str1, str2) = 0 Then
            Return True
        Else
            Return False
        End If
    Catch ex As Exception
    End Try
End Function

This VB .Net function could easily be replaced with String.Compare(queryEmail, FnsEmail) = 0. Of course, that’d be a little unclear, and since we only care about equality, we could just use String.Equals(queryEmail, FnsEmail)– which is honestly clearer than having a method called Mailid_compare, which doesn’t actually tell me anything useful about what it does.

Speaking of not doing anything useful, there are a few other pieces of bloat in this function.

First, we plop our input parameters into the variables str1 and str2, which does a great job of making what’s happening here less clear. Then we have the traditional “use an if statement to return either true or false”.

But the real special magic in this one is the Try/Catch. This is a pretty bog standard useless exception handler. No operation in this function throws an exception- String.Compare will even happily accept null references. Even if somehow an exception was thrown, we wouldn’t do anything about it. As a bonus, we’d return a null in that case, throwing downstream code into a bad state.

What’s notable in this case, is that every function was implemented this way. Every function had a Try/Catch that frequently did nothing, or rarely (usually when they copy/pasted from StackOverflow) printed out the error message, but otherwise just let the program continue.

And that’s the real WTF: a codebase polluted with so many do-nothing exception handlers that exceptions become absolutely worthless. Errors in the program let it continue, and the users experience bizarre, inconsistent states as the application fails silently.

Or, to put it another way: this is the .NET equivalent of classic VB’s On Error Resume Next, which is exactly the kind of terrible idea it sounds like.