I want to start doing TDD when doing anything new on a client project, or even when debugging.

I’m trying to use principles from “Working Effectively With Legacy Code” by Michael C Feathers, although I haven’t finished reading the book yet.

For this example I am validating a line for a Purchase Order form. There are two different criteria for whether the Quantity of the line is valid. I’ve already done the work, but I’m going to look at it again right now in terms of how I might have added tests. The original function IsLineValid is listed below:

Public Function IsLineValid() As Boolean
    IsLineValid = Not ISNULL(thisForm![Phase_Number]) And thisForm![Phase_Number] <> "" And thisForm![Phase_Number] <> 0 _
            And Not ISNULL(thisForm![Item_Description]) And thisForm![Item_Description] <> "" _
            And Not ISNULL(thisForm![Qty_Ordered]) And thisForm![Qty_Ordered] <> "" And thisForm![Qty_Ordered] <> 0 _
            And Not ISNULL(thisForm![Unit_of_Measure]) And thisForm![Unit_of_Measure] <> "" And ISNULL(thisForm!Cost_Type_ID) = False And ISNULL(thisForm![Taxable]) = False
    If IsLineValid = True And Nz(thisForm![Cost_Reference_Number], "") = "" Then Cancel_Cost_Reference_Check = False
End Function

In order to change this, I could have added addtional logic on line 3 of the big line continuation. Instead I chose to refactor it using multiple private functions to document better what I am doing to make it more understandable. This resulted in a lot more lines of code, but the original function validation piece is much simpler in terms of reading what pieces it’s validating. Then you can delve into the individual private functions to see how exactly it is validating it. Here is the code I just finished updating:

Public Function IsLineValid() As Boolean
    IsLineValid = IsPhaseValid _
            And IsItemDescriptionValid _
            And IsQtyValid _
            And IsUnitOfMeasureValid And IsCostTypeValid And IsTaxableValid
    If IsLineValid = True And Nz(thisForm![Cost_Reference_Number], "") = "" Then Cancel_Cost_Reference_Check = False
End Function

Private Function IsPhaseValid() As Boolean
    IsPhaseValid = Not ISNULL(thisForm![Phase_Number]) And thisForm![Phase_Number] <> "" And thisForm![Phase_Number] <> 0
End Function

Private Function IsItemDescriptionValid() As Boolean
    IsItemDescriptionValid = Not ISNULL(thisForm![Item_Description]) And thisForm![Item_Description] <> ""
End Function

Private Function IsQtyValid() As Boolean
    Dim retVal As Boolean
    If LineControlManager.LineIsConcrete Then
        retVal = IsQtyOrderedValid Or IsQtyUsedValid
    Else
        retVal = IsQtyOrderedValid
    End If
    IsQtyValid = retVal
End Function

Private Function IsQtyOrderedValid() As Boolean
    IsQtyOrderedValid = Not ISNULL(thisForm![Qty_Ordered]) And thisForm![Qty_Ordered] <> "" And thisForm![Qty_Ordered] <> 0
End Function

Private Function IsQtyUsedValid() As Boolean
    IsQtyUsedValid = Not ISNULL(thisForm![Qty_Used]) And thisForm![Qty_Used] <> "" And thisForm![Qty_Used] <> 0
End Function

Private Function IsUnitOfMeasureValid() As Boolean
    IsUnitOfMeasureValid = Not ISNULL(thisForm![Unit_of_Measure]) And thisForm![Unit_of_Measure] <> ""
End Function

Private Function IsCostTypeValid() As Boolean
    IsCostTypeValid = ISNULL(thisForm!Cost_Type_ID) = False
End Function

Private Function IsTaxableValid() As Boolean
    IsTaxableValid = ISNULL(thisForm![Taxable]) = False
End Function

So the IsLineValid Function which is public, is way easier to understand the business logic and uses domain language the customer would understand. All the isnull and <>”” checks are moved deeper into smaller functions.

The overall change I wanted to make happened in IsQtyValid which checks for the type of line and then checks the appropriate quantity fields for that type of line to see if the line is valid or not.

How could I have written tests for this?

Well, the only exposed function is the IsLineValid function. In order to test that, I would need to instantiate the whole class in a test, open a form with a line on it and set the values for the line. I have a big ugly dependency there which is the Form itself. I think in order to break the dependency and make it easier to test the new code itself, I would need to abstract the form away via an interface. So the thisForm object would need to not be the form I am testing, but a simulated object which would allow me to set the various field values being read. This would allow me to create a test and will make it easier to write future tests for this class. I think I will attempt to do that and report the results tomorrow. (I’m actually not sure I can use the Implements keyword in a Form, so that will make this a little more interesting).