Better understanding of unit testing in VBA from non-returning functions

Question

I have always wanted to start unit testing my code, and ending last week I downloaded Rubberduck again (first time there was just too little information how to do it to get me going) but after watching this video unit testing, I decided that it looks easy enough.

So the whole weekend I read up more and watched more videos and got more excited - because Monday I am going to go through my latest project and start incorporating unit tests!!

Or maybe not... I got to this first class of mine, and I really do not see a lot of places that I can do unit testing - meaning not a lot of return functions with a value to test.

The long and short of this class is that I inherited a worksheet with a lot of data all over the place, so much so that I can not even add or delete a line without breaking a lot of formulas (true story). Anyway, I made a new sheet, loaded information from a DB into that sheet, and from there on I am doing a data transfer to the infected patient (as mentioned above) every time I need to update the information.

My class with methods is below and I am wondering if there is anyway to run a test to check if data are transferred correctly, or to re-write the code to make it less brittle. Please feel free to tear the code apart and give any other advice you deem necessary.

Option Explicit
Private firstTruckColm As Long
Private truckRow As Long
Private numberOfShops As Long
Private numberOfProducts As Long
Private wb As Workbook
Private laailys As Worksheet
Private bloem As Worksheet
Private bloemShopNamesStartRow As Long
Private bloemShopNamesStartColm As Long

Public Sub loadTrucksInit()

    Call initValues
    Call insertShopNamesAndProd

End Sub

Private Sub initValues() 'Loads values from a "settings" module. (Any thoughts on this)'

    firstTruckColm = mdl_settings.returnLaailysFirstTruckColm
    truckRow = mdl_settings.returnLaailysTruckRow
    numberOfShops = mdl_settings.returnNumberOfBloemShops
    numberOfProducts = mdl_settings.returnNumberOfBloemProducts
    Set wb = ThisWorkbook
    Set laailys = wb.Sheets("Laailys")
    Set bloem = wb.Sheets("Bloem")
    bloemShopNamesStartRow = mdl_settings.returnBloemShopNamesStartRow
    bloemShopNamesStartColm = mdl_settings.returnBloemShopNamesStartColm

End Sub

Private Sub insertShopNamesAndProd()
    Dim a As Long
    For a = 1 To numberOfShops
        With laailys
            .Cells(truckRow, firstTruckColm + a - 1).Value = bloem.Cells(bloemShopNamesStartRow, bloemShopNamesStartColm + a).Value
        End With
        Call goThroughProducts(bloemShopNamesStartColm + a, firstTruckColm + a - 1)
    Next a
End Sub

Private Sub goThroughProducts(colmToUseBloem As Long, colmToUseLaailys As Long)
    Dim a As Long
    For a = bloemShopNamesStartRow + 1 To numberOfProducts + bloemShopNamesStartRow
        If bloem.Cells(a, colmToUseBloem).Value <> vbNullString Then
            Call insertProducts(bloem.Cells(a, 1).Value, bloem.Cells(a, colmToUseBloem).Value, colmToUseLaailys)
        End If
    Next a
End Sub

Private Sub insertProducts(productCode, prodAmount, colmToUseLaailys)
    Dim a As Long
    Dim checker As Boolean
    checker = False
    For a = 1 To 200
        If laailys.Cells(a, 4).Value = productCode Then
            laailys.Cells(a, colmToUseLaailys).Value = prodAmount
            checker = True
            Exit Sub
        End If
    Next a
    If checker = False Then
        MsgBox productCode & " did not read in correctly, make sure the product code in 'Laailys' is the same as in 'Bloem'"
    End If
End Sub

A few other questions that I also want to know if possible:

1. I like to have a initValues method that loads all the properties that I am going to use in my class all at once in one neat place if I need to change any. Is this an acceptable practice?

2. I point to colm values like firstTruckColm + a - 1, should I first declare another variable and give this value to that variable, just so that my arguments for other procedures look better?

3. Should I rather change my Subs to Functions with a return value as most I can?

Answer 1

You say this is a class, but you don't have anything exposed from it. What I mean is if I create an instance of this class, there's nothing I can do with it. I can't Get or Let any properties and I can't use any of the Private methods.

Call

You don't need to Call Sub anymore. You also don't need to enclose your arguments. E.g.

goThroughProducts somecolumn + a, somecolumn + a - 1

Worksheets

Worksheets have a CodeName property - View Properties window (F4) and the (Name) field (the one at the top) can be used as the worksheet name. This way you can avoid Sheets("mySheet") and instead just use mySheet.

Naming

Maybe some of this isn't in English and so I don't understand the meanings, but your naming could be improved. I have no idea what a laailys worksheet could be. Also you use product and prod both. Try to stay consistent and clear with your naming. What is checker checking? It's boolean so normally you would name it something like isMatch.

Also a Boolean is initialized as False so no need to set it as false.

Class

It seems strange to have a class refer to ThisWorkbook. What is your goal with this class? I don't see anything that can't be done with already existing objects.

ByVal

All your arguments are being passed ByRef implicitly. You want to pass ByVal whenever possible. e.g.

Private Sub InsertProducts(ByVal productCode As Long, ByVal productAmount As Long, ByVal targetColumn As Long

Typing

You'll notice I gave your variables a type above. Otherwise they are implicitly variants, which are not needed.

Loop

In InsertShopNamesAndProd you have a call to a procedure within a loop and a with:

For a = 1 To numberOfShops
        With laailys
            .Cells(truckRow, firstTruckColm + a - 1).Value = bloem.Cells(bloemShopNamesStartRow, bloemShopNamesStartColm + a).Value
        End With
        Call goThroughProducts(bloemShopNamesStartColm + a, firstTruckColm + a - 1)
    Next a

And in that procedure you have another call within a loop -

For a = bloemShopNamesStartRow + 1 To numberOfProducts + bloemShopNamesStartRow
        If bloem.Cells(a, colmToUseBloem).Value <> vbNullString Then
            Call insertProducts(bloem.Cells(a, 1).Value, bloem.Cells(a, colmToUseBloem).Value, colmToUseLaailys)
        End If
    Next a

That seems pretty repetitive. Instead think about refactoring these procedures so that you take all the products into an array, go through your shops and insert products as you work.

If

Here

If bloem.Cells(a, colmToUseBloem).Value <> vbNullString Then
            Call insertProducts(bloem.Cells(a, 1).Value, bloem.Cells(a, colmToUseBloem).Value, colmToUseLaailys)
        End If

Can be put on one line. And here

Dim checker As Boolean
    checker = False
    For a = 1 To 200
        If laailys.Cells(a, 4).Value = productCode Then
            laailys.Cells(a, colmToUseLaailys).Value = prodAmount
            checker = True
            Exit Sub
        End If
    Next a
    If checker = False Then
        MsgBox productCode & " did not read in correctly, make sure the product code in 'Laailys' is the same as in 'Bloem'"
    End If

I'm not sure how you can get to that second If without checker being false.

You can also use a boolean in an if as the actual test:

If Not checker Then

Settings module

You use your "settings" module to get objects and rows? That doesn't sound right. Settings should be loaded and stored so they can be reset. What you're doing is populating so naming it settings isn't entirely accurate.