These days I was doing a code review when I went across a method which contained a ReDim Preserve inside a loop.
I can’t show the real code because it is from a customer’s code base, but here’s a simplification of what I found:
Sub CopyAndPasteOrientedProgramming()
Dim c As New Customer()
For i As Integer = 0 To 250000
ReDim Preserve c.Orders(c.Orders.Length + 1)
c.Orders(c.Orders.Length – 1) = New Order()
Next
Console.WriteLine(c.Orders.Length)
End Sub
I found the developer who checked in the code and went on to tell him to use a List(Of Order) and he replied “Oh, don’t bother with that. It’s just a piece of code that I copied from a similar class and since it doesn’t have a business rule for adding the elements, I took off the If statement.
Wait a minute, I said. You’re telling me there’s more code like this scattered throughout the code base?
Yup, he said.
So to simplify, the business rule I’ll use will be the order being a multiple of 3:
Sub ShowRedim()
Dim c As New Customer()
For i As Integer = 0 To 250000
If i Mod 3 = 0 Then
ReDim Preserve c.Orders(c.Orders.Length + 1)
c.Orders(c.Orders.Length – 1) = New Order()
End If
Next
Console.WriteLine(c.Orders.Length)
End Sub
The problem with the code is that for each iteration of the loop, a new array will be created with one more element than earlier and the previous array will be copied into the new one. Yeap… It’s going to create 250,000 arrays on the first sample and 83,334 on the second. Poor Garbage Collector!
A smarter decision would be to use a generic List. If you don’t initialize its capacity, it’ll default to 16 when you add the first element. Then it will double in size each time it reaches its capacity. I’m too lazy to do the math but I guarantee you that it’s far less than the previous example.
Sub ShowList()
Dim c As New Customer()
Dim o As New List(Of Order)
For i As Integer = 0 To 250000
If i Mod 3 = 0 Then
o.Add(New Order)
End If
Next
c.Orders = o.ToArray()
Console.WriteLine(c.Orders.Length)
End Sub
This is a very simple alternative. If you really want to be clever and you have an idea of the percentage of orders that will fulfill the criteria, you can initialize the List with a meaningful capacity.
So please stop using ReDim Preserve inside loops!!!
PS. List(Of T).ToArray() creates a new array, but when compared with the original code that doesn’t hurt, does it?
