My refactoring heuristics (2)
“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: Subroutine
Heuristic: Initialization
Before
Public Function ReplaceEx(ByVal sSource, ByVal sPattern, ByVal sDest, ByVal boolMatchCase) Dim objRegExp Set objRegExp = new RegExp objRegExp.Global = TRUE objRegExp.IgnoreCase = Not boolMatchCase objRegExp.Pattern = sPattern ReplaceEx = objRegExp.Replace(sSource, sDest) Set objRegExp = Nothing End Function
Look for parameters that may cause false positives. Filter invalid values.
After
Public Function ReplaceEx(ByVal sSource, ByVal sPattern, ByVal sDest, ByVal boolMatchCase) Dim objRegExp If sPattern = "" Then ReplaceEx = sSource Exit Function End If Set objRegExp = new RegExp objRegExp.Global = TRUE objRegExp.IgnoreCase = Not boolMatchCase objRegExp.Pattern = sPattern ReplaceEx = objRegExp.Replace(sSource, sDest) Set objRegExp = Nothing End Function
Additional explanation. If sPattern = “” and sDest = “dolor” then sSource (passed in as “lorem ipsum”) will be transformed into “dolorldolorodolorrdoloredolormdolor doloridolorpdolorsdolorudolormdolor”. Did you really want to get that?
Heuristic: Duplication / Repetition
Before
'create XML node objRoot.AddChildElementByName "parent", "" Set objColl = objRoot.ChildElementsByPath("./parent") Set objXMLParent = objColl.Item(objColl.Count) 'create another XML node objXMLParent.AddChildElementByName "child", "" Set objColl = objXMLParent.ChildElementsByPath("./child") Set objXMLChild1 = objColl.Item(objColl.Count) 'create another XML node objXMLParent.AddChildElementByName "child", "" Set objColl = objXMLParent.ChildElementsByPath("./child") Set objXMLChild2 = objColl.Item(objColl.Count) 'create another XML node objXMLParent.AddChildElementByName "child", "" Set objColl = objXMLParent.ChildElementsByPath("./child") Set objXMLChild3 = objColl.Item(objColl.Count)
See if you can replace duplicated or repeating code blocks with another [reusable] subroutine with isolated functionality.
After
'------------------------------------------------------------------ 'Subroutine Public Function CreateChildElementByName(ByRef objXMLParent, ByVal sTagName, ByVal sInnerText) Dim objColl If objXMLParent is Nothing Then Set CreateChildElementByName = Nothing Exit Function End If objXMLParent.AddChildElementByName sTagName, sInnerText Set objColl = objXMLParent.ChildElementsByPath("./"&sTagName) If objColl.Count >0 Then Set CreateChildElementByName = objColl.Item(objColl.Count) Else Set CreateChildElementByName = Nothing End If End Function '------------------------------------------------------------------ 'Main code 'create XML nodes Set objXMLParent = CreateChildElementByName(objRoot, "parent", "") Set objXMLChild1 = CreateChildElementByName(objXMLParent, "child", "") Set objXMLChild2 = CreateChildElementByName(objXMLParent, "child", "") Set objXMLChild3 = CreateChildElementByName(objXMLParent, "child", "")
Heuristic: Parameter out of range
Before
Public Function GetTokenByNumber(ByVal strSource, ByVal chrDelimiter, ByVal intNumber, ByRef strToken) Dim Tokens Tokens = Split(strSource, chrDelimiter) strToken = Tokens(intNumber-1) End Function
For arrays, expand “happy path” logic with boundary conditions.
After
Public Function GetTokenByNumber(ByVal strSource, ByVal chrDelimiter, ByVal intNumber, ByRef strToken) Dim Tokens Tokens = Split(strSource, chrDelimiter) If intNumber > UBound(Tokens) Then Exit Function End If strToken = Tokens(intNumber-1) End Function
Heuristic: Exit Value
Before
Public Function GetTokenByNumber(ByVal strSource, ByVal chrDelimiter, ByVal intNumber, ByRef strToken) Dim Tokens Tokens = Split(strSource, chrDelimiter) If intNumber > UBound(Tokens) Then Exit Function End If strToken = Tokens(intNumber-1) End Function
Avoid returning from subroutine with an uncertain value. Assign either “” or zero, or valid exit code.
After
Public Function GetTokenByNumber(ByVal strSource, ByVal chrDelimiter, ByVal intNumber, ByRef strToken) Dim Tokens Tokens = Split(strSource, chrDelimiter) If intNumber > UBound(Tokens) Then strToken = "" GetTokenByNumber = FALSE Exit Function End If strToken = Tokens(intNumber-1) GetTokenByNumber = TRUE End Function
Heuristic: Hard-Coding
Before
Public Function Add2List(ByVal sList, ByVal sItem, ByVal sDelimiter) If sItem = "" Then Add2List = sList Exit Function End If If sDelimiter = "" Then sDelimiter = "," End If If sList = "" Then Add2List = sItem Else Add2List = sList & sDelimiter & sItem End If End Function
If a subroutine uses hard-coded values which retain the same meaning outside of the context (of the subroutine) it might worth mapping them to constants.
After
'Declare constant Private Const chrListSeparator = "," ' Public Function Add2List(ByVal sList, ByVal sItem, ByVal sDelimiter) If sItem = "" Then Add2List = sList Exit Function End If If sDelimiter = "" Then sDelimiter = chrListSeparator End If If sList = "" Then Add2List = sItem Else Add2List = sList & sDelimiter & sItem End If End Function