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?

• Alfa Bravo: Also, a few weeks ago I read a post by RubberDuck (I think) where he explained how to refracture an example that he gave of where you get data from an Access DB and use it in a sheet (or vice versa) in such a way that you can unit test it, where you couldn't do it with the original code. I can not find it for the life of me now, if someone knows what I am talking about please link it for me. I immediately like the idea when I read it.
• Raystafarian: Maybe you're thinking of rubberduck vba, a project that includes unit testing and has documentation on how to use it? One of the developers is rubberduck. mat also has a lot of answers in that realm
• Raystafarian: Oh, Thomas posts some pretty good ideas like what you mention as well.
• Alfa Bravo: Hi Raystafarian, I was def thinking of RDVBA and I am using it at the moment, but I looked every where on the site and related documentation to see if I could find it again. In the end I realized I must have looked at the page at home and not at work, so when I went through search history there I got it: [How to unit test][1]. [1]: rubberduckvba.wordpress.com/2017/10/19/…
• Raystafarian: You can also visit the wiki - github.com/rubberduck-vba/Rubberduck/wiki/Unit-Testing

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.

• Alfa Bravo: Hi Ray, first of all thank you for the time you have put into answering my question - I am systemically going through every point you have made and will leave a comments as I go. Firstly, on you first question, I run the class in my form (or the press of a button) by calling the public loadTrucksInit method, which then runs everything in the class and does what I want it to do. I think my big problem is still that I may be using classes just like modules - is that what it looks like to you too?
• Raystafarian: Ah, on a form. Yeah, Using a Class is like creating an Object (like Range, Variant, Dictionary). So if there's already an object that does everything you need, there's no real need for a class. You can still make the class, but usually it's to combine several different properties into one object.