How could I rewrite this to remove the gotos without loss of speed or readability?

Question

I wrote this. Yes, I know it's VB6. Yes, it is production code, and, yeah, I know it uses gotos. I am a lazy, evil beast ...

So show me (and the rest of us) how it should be written

Public Function SplitString(ByVal sText As Variant) As Variant
    Dim nHere As Long
    Dim cHere As String * 1
    Dim aRes As Variant
    Dim nRes As Long
    Dim bInquote As Boolean
    Dim sString As String
    ReDim aRes(0)
    nHere = 1
    nRes = 0
    Do
 If nHere > Len(sText) Then Exit Do
 cHere = Mid$(sText, nHere, 1)
 If cHere = Chr$(32) Then
     If bInquote Then
  sString = sString & cHere
  GoTo nextChar
     End If
     If sString <> vbNullString Then
  aRes(nRes) = sString
  sString = vbNullString
  nRes = nRes + 1
  ReDim Preserve aRes(nRes)
     End If
     GoTo nextChar
 ElseIf cHere = Chr$(34) Then
     bInquote = Not bInquote
     GoTo nextChar
 Else
     sString = sString & cHere
 End If
nextChar:
 nHere = nHere + 1
    Loop
    If sString <> vbNullString Then
 aRes(nRes) = sString
    End If
    SplitString = aRes
End Function

By the way, it splits a string into an array. The elements in the string may be quoted.

Answer 1

It's pretty simple:

Change "If sString <> vbNullString Then" to "ElseIf sString <> vbNullString Then", remove all "Goto"s and remove "nextChar:".

Public Function SplitString(ByVal sText As Variant) As Variant
    Dim nHere As Long
    Dim cHere As String * 1
    Dim aRes As Variant
    Dim nRes As Long
    Dim bInquote As Boolean
    Dim sString As String
    ReDim aRes(0)
    nHere = 1
    nRes = 0
    Do
        If nHere > Len(sText) Then Exit Do
        cHere = Mid$(sText, nHere, 1)
        If cHere = Chr$(32) Then
            If bInquote Then
                sString = sString & cHere
            ElseIf sString <> vbNullString Then
                aRes(nRes) = sString
                sString = vbNullString
                nRes = nRes + 1
                ReDim Preserve aRes(nRes)
            End If
        ElseIf cHere = Chr$(34) Then
            bInquote = Not bInquote
        Else
            sString = sString & cHere
        End If

        nHere = nHere + 1
    Loop
    If sString <> vbNullString Then
        aRes(nRes) = sString
    End If
    SplitString = aRes
End Function

Answer 2

Looks pretty straight forward. Just make use of the if/else structure. If your code enters an if block, it will never enter corresponding elseif or else blocks. A lot of your goto statements aren't even needed in the first place.

Public Function SplitString(ByVal sText As Variant) As Variant
    Dim nHere As Long
    Dim cHere As String * 1
    Dim aRes As Variant
    Dim nRes As Long
    Dim bInquote As Boolean
    Dim sString As String
    ReDim aRes(0)
    nHere = 1
    nRes = 0
    Do
        If nHere > Len(sText) Then Exit Do
        cHere = Mid$(sText, nHere, 1)
        If cHere = Chr$(32) Then
            If bInquote Then
                sString = sString & cHere
            ElseIf sString <> vbNullString Then
                aRes(nRes) = sString
                sString = vbNullString
                nRes = nRes + 1
                ReDim Preserve aRes(nRes)
            End If
        ElseIf cHere = Chr$(34) Then
            bInquote = Not bInquote
        Else
            sString = sString & cHere
        End If
        nHere = nHere + 1
    Loop
    If sString <> vbNullString Then
        aRes(nRes) = sString
    End If
    SplitString = aRes
End Function

Answer 3

I'd simply use the Split function and Join functions that is native to VB6. I'll leave the details to you but the basic flow is to split on the spaces and then rejoin objects that are between the quotes.

I've not written VB6 in years and I don't have the software installed (for debugging) so I don't want to take a stab at the function.

Best Luck,
Frank

Answer 4

I heard a rule of thumb that says that if your goto jumps forward, it's probably okay. This looks like one of those cases. If you can reverse the logic of a predicate, though, seems easier.

Answer 5

I agree that this particular bit of logic should be clearly and easily implemented using Split() and Join() operations. While one can always write a long run of inline code that improves on them in speed there are two reasons not to:

• The performance difference probably will not even approach a factor of 1.5 and such logic is seldom used millions of times on extremely large strings.
• Such inline logic is not only opaque and difficult to maintain, it is hard to get right the first time when writing the program.

Example:

Function SplitString(ByVal Text As String) As String()
    Dim Slices() As String
    Dim UnquotedSlice As Long

    Slices = Split(Text, """")
    For UnquotedSlice = 0 To UBound(Slices) Step 2
        Slices(UnquotedSlice) = Replace$(Slices(UnquotedSlice), " ", vbNullChar)
    Next
    SplitString = Split(Join$(Slices, ""), vbNullChar)
End Function

BTW: Kudos to anyone who can fix the perverse code-quoting markup this site uses in my example above.

Edit: Nevermind, I winkled it out. The bulletted list gave the parser a spasm.

Answer 6

You should have a look here, for an idea of how subtle optimizations affect this sort of task:

http://www.xbeat.net/vbspeed/c_Split.htm

You'll see that amongst those algorithms, there is no best one, but rather some pretty good ones and then some that can really whoop the outlyers a bit better than others.