My refactoring heuristics (1)

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: 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:

  1. separating and isolating from each other data validation and data processing functionality;
  2. separating functionalities per clause;
  3. 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

  • Trackbacks

  • Trackback fromOpenQuality.ru | Качество программного обеспечения
    Thursday, 1 July, 2010

    […] проводит уроки рефакторинга, призванного повысить надежность программного кода (1, 2, 3). […]

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