preload

My refactoring heuristics (2)

Posted by Albert Gareev on Jun 15, 2010 | Categories: ImplementationNotes

“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. “

Martin Fowler

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


Creative Commons Attribution-NonCommercial-NoDerivs 3.0 Unported
This work by Albert Gareev is licensed under a Creative Commons Attribution-NonCommercial-NoDerivs 3.0 Unported.