XNSIO
  About   Slides   Home  

 
Managed Chaos
Naresh Jain's Random Thoughts on Software Development and Adventure Sports
     
`
 
RSS Feed
Recent Thoughts
Tags
Recent Comments

GOTOs Condisered Harmful! But…

Original Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
Public Function Process(ByVal headers As HeaderCollection, ByVal factory As HeadersFactory) As Boolean
    On Error GoTo ErrorHandler
    Dim i As Long
    Dim header As Header
    Dim receivedHeader As ReceivedHeader
    For i = 1 To headers.Count
        Set header = headers.Item(i)
        If (Trim(header.HeaderKey) = "Received") Then
            Set receivedHeader = factory.CreateReceivedHeader(header)
            If Not receivedHeader.IsIntranetServer Then
                If receivedHeader.IsTrustedRecipient Then
                    headers.ourHeadersExistBefore (i)
                    Exit For
                End If
            End If
        End If
    Next
    Exit Function
ErrorHandler:
    Err.Raise Err.Number, Err.Source & vbCrLf & "HeadersProcessor.Process", Err.Description
1
End Function

The nested if-else blocks bother me. Also the code does not communicate what is happening clearly.

Refactored Code With Goto:

1
2
Private receivedHeader As ReceivedHeader
Private recievedHeaderIndex As Long
1
2
3
4
5
6
7
8
9
10
Public Function Process(ByVal headers As HeaderCollection, ByVal factory As HeadersFactory) As Boolean
    On Error GoTo ErrorHandler
    Call ExtractFirstReceivedHeader(headers, factory)
    If receivedHeader Is Nothing Then Exit Function
    If receivedHeader.IsTrustedRecipient Then
        headers.ourHeadersExistBefore (recievedHeaderIndex)
    End If
    Exit Function
ErrorHandler:
    Err.Raise Err.Number, Err.Source & vbCrLf & "HeadersProcessor.Process", Err.Description
1
End Function
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
Private Sub ExtractFirstReceivedHeader(ByVal headers As HeaderCollection, ByVal factory As HeadersFactory)
    On Error GoTo ErrorHandler
    Dim i As Long
    Dim header As Header
    For i = 1 To headers.Count
        Set header = headers.Item(i)
        If (Trim(header.HeaderKey) <> "Received") Then <strong>GoTo Continue</strong>
        If NotAnIntranetServer(header, i, factory) Then GoTo FinishedProcessing
 
<strong>Continue</strong>:
    Next
 
FinishedProcessing:
    Set header = Nothing
    Exit Sub
1
2
ErrorHandler:
    Err.Raise Err.Number, Err.Source & vbCrLf & "HeadersProcessor.SetFirstReceivedHeader", Err.Description
1
End Sub
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
Private Function NotAnIntranetServer(ByVal header As Header, currentIndex As Long, ByVal factory As HeadersFactory) As Boolean
    On Error GoTo ErrorHandler
    Set receivedHeader = factory.CreateReceivedHeader(header)
    If receivedHeader.IsIntranetServer = False Then
        recievedHeaderIndex = currentIndex
        NotAnIntranetServer = True
    Else
        Set receivedHeader = Nothing
        NotAnIntranetServer = False
    End If
 
Exit Function
ErrorHandler:
    Err.Raise Err.Number, Err.Source & vbCrLf & "HeadersProcessor.NotAnIntranetServer", Err.Description
End Function
1
2
3
Private Sub Class_Terminate()
    Set receivedHeader = Nothing
End Sub

    Licensed under
Creative Commons License