DevDisasters

The Regex Code Review

• By Mark Bowytz
• 02/01/2011

Ding! A new e-mail appeared in Jed's and the other developers' inboxes on the floor. It was from his coworker Bob, and it read:

"Jed, the term-validation process of CAPBACS is a performance bottleneck that's costing our company thousands in lost sales every month. This change must go in ASAP. While I appreciate your feedback and input, I don't believe that your experience working on the Web places you in a position to critique my work. Thanks!"

Before a change could be promoted, the team had to send out the code for comments to a mailing list of fellow developers.

A Stackable Solution
Bob had sent out a single VB.NET function with an accompanying class definition. Its sole purpose in life was to see if at least one of the terms, passed as an array of strings, existed in the second parameter, a document body. Jed was positive that somewhere, a Reverse Polish Notation calculator was crying:

Public Function DocContainsAtLeastOneTerm(ByVal   
  searchTerms() As String, ByVal searchText As   
    String) As Boolean

      Dim paramTable As New StackType
      Dim searchTable As New StackType
      Dim truthTable As New StackType

      For i As Integer = 
        0 To searchTerms.Length - 1 Step 1
        paramTable.Push(searchTerms(i).ToLower())
      Next

      For i As Integer = 
        0 To searchText.Split(" ").Length - 1 Step 1
        searchTable.Push(searchText.Split(" ") 
          (i).ToLower())
      Next

      For i As Integer = 
        1 To paramTable.stackHeight() - 1 Step 1
        If searchTable.Contains(paramTable.Pop()) Then
          truthTable.Push(True)
        Else
          truthTable.Push(False)
        End If
      Next

      If truthTable.stackHeight() > 0 And 
        truthTable.Contains(True) Then
        Return True
      Else
        Return False
      End If
    End Function

Jed had sent a private e-mail explaining that while Bob's implementation was viable and would surely work, there had to be a better way that didn't involve recreating built-in stack functionality.

Rather than try to clean up Bob's approach, Jed showed him just what an "inexperienced" Web developer could do:

Public Function DocContainsAtLeastOneTermNew(
  ByVal searchTerms() As String, ByVal searchText  
  As String) As Boolean
      If searchTerms.Length = 0 Then
        Return False
      End If

      Dim regexString As String = 
        "(" + searchTerms(0)

      If searchTerms.Length 
        > 1 Then
        For i As Integer = 1 To searchTerms.Length - 1 Step 1
          regexString += "|" + 
            searchTerms(i)
        Next
      End If

      regexString += ")"

      Dim rx As Object = New Regex(regexString)
      Dim matches As MatchCollection 
        = rx.Matches(searchText)

      If matches.Count > 0 Then
        Return True
      Else
        Return False
      End If
    End Function

Maybe Next Time
Positive his code would prevail, Jed sent a polite message back to Bob and the rest of his coworkers on the mailing list, suggesting that regular expressions might be a better approach. He attached his sample code, details of the tests he ran and the results, and sat back, quite smug that his skills in applied one-upmanship had prevailed.

In the end, Bob and Jed's group manager stepped in and declared they'd be moving forward with Bob's version, and save Jed's version for a "2.0" revision. The department couldn't simply discard Bob's effort. That would be a waste of time and money.