preload

My refactoring heuristics (1)

Posted by Albert Gareev on Jun 08, 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: 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

  • Leave a Reply

    * Required
    ** Your Email is never shared

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.