Option Explicit
Always nice to see.
Public Function RemoveBlanksFromArray(ByVal TheArray As Variant) As Variant
The usual convention for parameters is camelCase - i.e. theArray. Otherwise looks fine, maybe I'd call it inputArray so it's obvious to the rest of your code what it contains (more on naming).
One thing to watch out for when passing arrays byVal. Arrays are a bit like objects in VBA -you never pass actual objects around, just addresses. Similarly, byVal doesn't make a copy of the actual data in TheArray, just a copy of the pointer to the array's location in memory. With primitive types when you write something like this:
Sub Foo(ByVal bar As Long)
... only a copy of bar is passed, meaning Foo cannot do anything to alter the original variable. However in your code, you could accidentally modify the values in TheArray.
This is one reason why I'd suggest strongly typing the array - as these can only be passed ByRef; in your comparisons you treat the elements like strings:
If myElement <> "" Then
So why not explicitly declare the array as such?
Public Function RemoveBlanksFromStringArray(ByRef theArray() As String) As String()
Now the ByRef is clear, and you'll get a minor performance boost because you don't have to cast to and from Variant
Dim temp As Variant
'[...]
RemoveBlanksFromArray = temp
I tend to name variables that get assigned to the return value of a function result, just personal preference.
ReDim temp(LBound(TheArray) To LBound(TheArray))
Glad to see you've explicitly referenced both bounds, avoiding Option Base 1 or any convention (hint, there isn't one)
Dim myCount As Long
I think this name is misleading. It's not a count really is it, because depending on LBound(TheArray) it might start at 0 or 1 or 7 for all we know. You could name it something like indexOfTempArray, but I think a count is actually more useful and intuitive, so we'll make it one, but maybe more explicit:
Dim countOfNonBlanks As Long
For Each myElement In TheArray
Now there's a small optimisation to be made here; it's marginally quicker to loop over an array by index than with For-Each, so this becomes
Dim index As Long
For index = LBound(TheArray) to UBound(TheArray)
If TheArray(index) <> "" Then
'...
Oh and while we're here, prefer vbNullString to "" because the latter could be a non printable character or might have a space in there if you squint etc. vbNullString is unequivocal (Rubberduck would've told you that)
That all brings us to the bottle-neck of your code (I think):
ReDim Preserve temp(LBound(temp) To myCount)
temp(myCount) = myElement
myCount = myCount + 1
Your hunch is right, there's a better way. Arrays in VBA are stored as a continuous pre-allocated bit of memory. (This is why array lookups are so fast, because all you do is look up the address of the first item of the array, and offset by a fixed distance - 1 lookup operation. Collections store the address of every element of the array, so to find an element in memory, you first lookup the address associated with it, then look at that address in memory - 2 lookup operations).
However with VBA Arrays what you gain in speed you lose in flexibility. Because the memory is pre-allocated, you can't just add another element on the end, that memory may be in use for something else. So ReDim Preserve actually copies the entire array to a new (bigger) location in memory. That's slow! Here's where someone told me all that.
Anyway, what this boils down to is; imagine the largest size your temp array could ever be (i.e no blanks found, so the same size as your input array), fill it up partially, then ReDim Preserve it once back down to the actual size it has to be.
Putting all that together we get:
Public Function RemoveBlanksFromStringArray(ByRef inputArray() As String) As String()
Dim base As Long
base = LBound(inputArray)
Dim result() As String
ReDim result(base To UBound(inputArray))
Dim countOfNonBlanks As Long
Dim i As Long
Dim myElement As String
For i = base To UBound(inputArray)
myElement = inputArray(i)
If myElement <> vbNullString Then
result(base + countOfNonBlanks) = myElement
countOfNonBlanks = countOfNonBlanks + 1
End If
Next i
If countOfNonBlanks = 0 Then
ReDim result(base To base)
Else
ReDim Preserve result(base To base + countOfNonBlanks - 1)
End If
RemoveBlanksFromStringArray = result
End Function
On a test of 500,000 items
- original code took 1.1481±0.0001 s,
- refactored code took 0.1157±0.0001 s
or ~ 10 times faster
Addendum
Now I think about it, the original code with copying memory boils down to an \$\mathcal O (n^2)\$ algorithm, the refactored code is \$\mathcal O (n)\$ (here \$ n\$ is the size of the array, \$\mathcal O (n^a)\$ basically means you loop over the array \$ a\$ times). Using Timer for some rough results, you can see this trend:
10,000 elements
Old 0
New 0
100,000 elements
Old 0.046875 (same order of magnitude as each other)
New 0.015625
1,000,000 elements
Old 3.171875 (1 order of magnitude slower relative to New)
New 0.125
10,000,000 elements
Old 321.46875 (2 orders of magnitude slower)
New 1.3125
I.e. When you do 10x more elements, OP's O(n^2) code gets 10^2 = 100 times slower, the refactored O(n) gets 10^1 = 10 times slower. Therefore relative to the new code, the old code gets 10x slower.
Interestingly, because both algorithms are doing essentially the same operation (writing to memory), which is an O(1) operation (i.e. independent of the rest of the code), once you operate on large enough arrays, optimising the rest of the code becomes inconsequential (Early vs Late binding, For Each vs Index, re-using LBound vs re-measuring). So once you get the algorithm down to the lowest complexity possible (assuming performance is an issue), then pick whichever method is most readable and maintainable.
