ArrayList.Add with Object Creates Chaos

Question

I'm sorry for the broad characterization of "chaos" in the title, but I really don't know how else to describe it. What I'm trying to do is build a macro in Excel to parse and organize a large number of financial entries from one file into another. To do that, I first read in the entries, and debug output to that stage seems to indicate that the parsing up to the point is fine. But then I was putting them in an ArrayList and when I tried to read them back out the behavior was totally unexpected. When I added a breakpoint and tried to watch the process in the Local window, I saw that sometimes the number of entries would increment while other times it wouldn't, and frequently it looked like the newest entry was overwriting all the previous entries.

Below is a simplified example that re-creates the problem. What I think that this should do is create a worksheet titled "ADAM WEST" with ten charge entries with the hours incrementing from 1-10. What it actually does is create the sheet just fine, but with only one entry of 10 hours.

Sub DebugTest()
    Dim task As New sTask
    Dim this_charge As New sCharge
    For i = 1 To 10
        this_charge.wbs_element = this_wbs
        this_charge.nwa = "NWA123456"
        this_charge.nwa_title = "TEST NWA"
        this_charge.name = "ADAM WEST"
        this_charge.post_date = "03/15/2024"
        this_charge.hours = i
        this_charge.log_date = "03/14/2024"
        this_charge.labor_cost = "100"
        Call task.AddLaborCharge(this_charge)
    Next i
    task.ProcessLaborCharges
End Sub

' Class Module: sTask

Dim laborcharge_array As New ArrayList

Public Sub AddLaborCharge(charge As sCharge)
    laborcharge_array.Add charge
End Sub

Public Sub ProcessLaborCharges()
    Dim charge As sCharge
    Dim employee As sEmployee
    For Each charge In laborcharge_array
        Set employee = GetEmployee(charge.name)
        Dim employee_charges As Worksheet
        Set employee_charges = ThisWorkbook.Worksheets(charge.name)
        Dim next_row As Integer
        next_row = employee_charges.Cells(Rows.count, 1).End(xlUp).Row + 1
        employee_charges.Cells(next_row, 1).Value = charge.wbs_element
        employee_charges.Cells(next_row, 2).Value = charge.nwa
        employee_charges.Cells(next_row, 3).Value = charge.nwa_title
        employee_charges.Cells(next_row, 4).Value = charge.hours
        employee_charges.Cells(next_row, 5).Value = charge.labor_cost
        employee_charges.Cells(next_row, 6).Value = charge.log_date
        employee_charges.Cells(next_row, 7).Value = charge.post_date
    Next charge
End Sub

I'm much more comfortable with C++ and Python, so it's very likely I'm missing something obvious in the VBA world.

Answer 1

In DebugTest(), the task.AddLaborCharge() method does NOT make a new object or copy the data from the this_charge variable. Rather, it adds a reference to the same single object created at the beginning.

The code then loops several times, mutating this same object and adding a reference (not a copy!) to the collection each time, so at the end you have a list with a bunch of references to the same object that has been changed a bunch of times... but all the references point to the same item that now matches the values from the final loop iteration.

Instead, you should create a new instance of the sCharge object for each iteration inside the loop:

Sub DebugTest()
    Dim task As New sTask   
    Dim this_charge As sCharge 
    For i = 1 To 10
        Set this_charge = new sCharge
        this_charge.wbs_element = this_wbs
        this_charge.nwa = "NWA123456"
        this_charge.nwa_title = "TEST NWA"
        this_charge.name = "ADAM WEST"
        this_charge.post_date = "03/15/2024"
        this_charge.hours = i
        this_charge.log_date = "03/14/2024"
        this_charge.labor_cost = "100"
        Call task.AddLaborCharge(this_charge)
    Next i
    task.ProcessLaborCharges
End Sub

---

It's worth nothing we also have this code inside the AddLaborCharge() method:

laborcharge_array.Add charge

VB has a long-standing convention of using parentheses (or not) for method arguments to control whether they are passed by reference or by value. However, if the sCharge type is a reference type... ie a Class rather than a Structure... then even when passed by value, the value of the variable is still just a reference.