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
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()
ByRef is clear, and you'll get a minor performance boost because you don't have to cast to and from
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
"" 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
If countOfNonBlanks = 0 Then
ReDim result(base To base)
ReDim Preserve result(base To base + countOfNonBlanks - 1)
RemoveBlanksFromStringArray = result
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
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:
Old 0.046875 (same order of magnitude as each other)
Old 3.171875 (1 order of magnitude slower relative to New)
Old 321.46875 (2 orders of magnitude slower)
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.