My refactoring heuristics (1)
“Refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior. Its heart is a series of small behavior preserving transformations. Each transformation (called a ‘refactoring’) does little, but a sequence of transformations can produce a significant restructuring. Since each refactoring is small, it’s less likely to go wrong. The system is also kept fully working after each small refactoring, reducing the chances that a system can get seriously broken during the restructuring. “
Test Automation / VBScript / Refactoring
Subject: Code Block
Heuristic: Exit Point
Before
boolRC = objRegExp.Test(sSource) If Not boolRC Then Regex_Match = "" Exit Function End If
Look for created and not released local object instances nearby an exit point.
After
boolRC = objRegExp.Test(sSource) If Not boolRC Then Set objRegExp = Nothing Regex_Match = "" Exit Function End If
Heuristic: Error-handling
Before
boolRC = objRegExp.Test(sSource) If Not boolRC Then Set objRegExp = Nothing Regex_Match = "" Exit Function End If
Whenever encountering an external call that might break execution or raise exception see if you can improve it with error-handling.
After
On Error Resume Next boolRC = objRegExp.Test(sSource) intRC = Err.Number On Error GoTo 0 If intRC <> 0 Then boolRC = FALSE If Not boolRC Then Set objRegExp = Nothing Regex_Match = "" Exit Function End If
Heuristic: “Used Once” Objects
Before
Set FSO = CreateObject("Scripting.FileSystemObject")
boolRC = FSO.FileExists(sFileName)
If Not boolRC Then
DT_ImportSheet = Nothing
Exit Function
End If
DataTable.AddSheet sSheetDest
DataTable.ImportSheet sFileName, SheetSource, sSheetDest
Look if an object is used in one step and never referred thereafter.
After
Set FSO = CreateObject("Scripting.FileSystemObject")
boolRC = FSO.FileExists(sFileName)
Set FSO = Nothing
If Not boolRC Then
DT_ImportSheet = Nothing
Exit Function
End If
DataTable.AddSheet sSheetDest
DataTable.ImportSheet sFileName, SheetSource, sSheetDest
Heuristic: Stacked Conditions
Before
If objDictionary.Exists("DefaultAmount") Then
If isNumeric(objDictionary.Item("DefaultAmount")) Then
If CInt(objDictionary.Item("DefaultAmount")) >= 10 Then
'example of calculations
TransactionAmount = CInt(objDictionary.Item("DefaultAmount")) + FeeAmount
End If
End If
End If
Any stacked (more than 2 enclosed statements) conditional code is harder to trace, debug, and maintain.
To simplify code structure, consider following:
- separating and isolating from each other data validation and data processing functionality;
- separating functionalities per clause;
- defining additional exit points for invalid or special cases.
After (1)
If objDictionary.Exists("DefaultAmount") Then
DefaultAmount = objDictionary.Item("DefaultAmount")
Else
DefaultAmount = 10
End If
If isNumeric(DefaultAmount) Then
DefaultAmount = CInt(DefaultAmount)
Else
DefaultAmount = 10
End If
If DefaultAmount >= 10 Then
'example of calculations
TransactionAmount = CInt(objDictionary.Item("DefaultAmount")) + FeeAmount
End If
After (2)
If Not objDictionary.Exists("DefaultAmount") Then
TransactionAmount = GetDefaultTransaction()
End If
If Not isNumeric(objDictionary.Item("DefaultAmount")) Then
TransactionAmount = GetDefaultTransaction()
End If
If isNumeric(objDictionary.Item("DefaultAmount")) Then
TransactionAmount = GetRegularTransaction(CInt(objDictionary.Item("DefaultAmount")))
End If
After (3)
If Not objDictionary.Exists("DefaultAmount") Then
TransactionAmount = -1
Exit Function
End If
If Not isNumeric(objDictionary.Item("DefaultAmount")) Then
TransactionAmount = -1
Exit Function
End If
DefaultAmount = CInt(objDictionary.Item("DefaultAmount"))
If DefaultAmount >= 10 Then
'example of calculations
TransactionAmount = DefaultAmount + FeeAmount
Exit Function
End If
Heuristic: Logical Boundaries
Before
Select Case intStatus Case micPass Status2Str = "PASS" Case micFail Status2Str = "FAIL" Case micDone Status2Str = "OK" Case micWarning Status2Str = "WARNING" End Select
Look for open boundaries in conditions. Close with explicit values.
After
Select Case intStatus Case micPass Status2Str = "PASS" Case micFail Status2Str = "FAIL" Case micDone Status2Str = "OK" Case micWarning Status2Str = "WARNING" Case Else Status2Str = "UNKNOWN" End Select

