5
\$\begingroup\$

I've written a function to remove empty or "" value elements from an array or in other words return an array consisting only of elements containing a value.

Option Explicit

Public Function RemoveBlanksFromArray(ByVal TheArray As Variant) As Variant

    Dim temp As Variant
    ReDim temp(LBound(TheArray) To LBound(TheArray))
    Dim myElement As Variant
    Dim myCount As Long
    myCount = LBound(temp)

    For Each myElement In TheArray
        If myElement <> "" Then
            ReDim Preserve temp(LBound(temp) To myCount)
            temp(myCount) = myElement
            myCount = myCount + 1
        End If
    Next myElement

    RemoveBlanksFromArray = temp

End Function

The Function is passed an Array as an argument. The Array is temp and ReDimed as a single element. It then loops each element of the passed argument and adds the value if it's not "" or Empty.

I've used ReDim Preserve temp(LBound(temp) To myCount) before each element is written to temp to dynamically set the upper bound.

I feel there may be more efficient ways to perform this task.

Are there any inefficiencies in ReDiming the upper bound the way I have done it?

Can the upper bound of the array be set in any other way dynamically?

Have I broken any sacred rules in the formatting of the function or naming conventions etc?

\$\endgroup\$
6
\$\begingroup\$
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.

\$\endgroup\$
  • \$\begingroup\$ Nice answer. You only need to Redim the array If countOfNonBlanks > 0. \$\endgroup\$ – Rick Davin Mar 25 at 16:47
  • \$\begingroup\$ @RickDavin Thanks! Are you sure? Because ReDim result(base To UBound(inputArray)) is one of the first moves, before we know countOfNonBlanks. If we wan't to emulate OP's code and return a 1 element empty array if the input array is entirely blank, ReDim is needed for countOfNonBlanks > 0 surely? ReDim Preserve is for when countOfNonBlanks > 0. Or am I missing something obvious (wouldn't be the first time ;) \$\endgroup\$ – Greedo Mar 25 at 16:58
  • \$\begingroup\$ @Greedo what are you using to get the time comparisons between each version? \$\endgroup\$ – IvenBach Mar 25 at 17:22
  • \$\begingroup\$ @IvenBach My StopWatch class for the timings, testArray(1 To 500000) As String filled with ~50% blanks, 50% 10-20 char text. Why, are you not getting similar results, or just out of curiosity? Do you think I should post the test code, it's quite straightforward but fairly verbose because I've used RubberDuck unit tests (OTT) \$\endgroup\$ – Greedo Mar 25 at 17:27
  • \$\begingroup\$ Pure curiosity. I'd seen your previous work towards benchmarking. TL;DR = your refactoring blew mine out of the water speed-wise, so much so I'm ashamed to show it. \$\endgroup\$ – IvenBach Mar 25 at 17:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.