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 |
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
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 |
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 |
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
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 |
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 |
ErrorHandler:
Err.Raise Err.Number, Err.Source & vbCrLf & "HeadersProcessor.SetFirstReceivedHeader", Err.Description
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 |
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 |
Private Sub Class_Terminate()
Set receivedHeader = Nothing
End Sub
This entry was posted
on Monday, February 9th, 2009 at 3:52 PM and is filed under Agile, Testing.
You can follow any responses to this entry through the RSS 2.0 feed.
You can leave a response, or trackback from your own site.