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