I'm doing VBA macros and need to use Arrays of values extensively. After the below help from @TonyDallimore on Stack Overflow, I've decided to use nested variant arrays.
VBA chrashes when trying to execute generated sub
I use multidimensional, jagged arrays to SELECT
data from DB's and efficiently perform calculations and write to a Worksheet. The data is sometimes a single value (Rank:0) sometimes a row of values (Rank:1) sometimes a table of values with some cells containing rows of values (Rank:3). I use the function below to determine what kind of operations are possible and should be performed to such arrays.
This function, along with all my array related functions reside in a module: modCustomArrayFunctions.
'***************************************************************************'
'Returns the rank of the passed array '
'Parameters: '
' Arr: The array to be processed '
'Returns: '
' The rank of the array '
'***************************************************************************'
Public Function Rank(ByRef Arr As Variant) As Byte
'Declarations **************************************************************'
Dim MaxRank As Byte 'Maximum rank of the elements of the array '
Dim i As Integer
'***************************************************************************'
If IsArray(Arr) Then
If IsArrInitialized(Arr) Then
MaxRank = 0
For i = LBound(Arr) To UBound(Arr)
Rank = Rank(Arr(i)) + 1
If Rank > MaxRank Then MaxRank = Rank
Next i
Rank = MaxRank
Else
Rank = 0
End If
Else
Rank = 0
End If
End Function
'***************************************************************************'
I code my routines pretty much this way.
And the accompanying test is as follows:
'***************************************************************************'
Public Sub Rank_Test()
Dim TestArr As Variant
Dim TestRank As Integer
'Test Non-Array
If Rank(TestArr) = 0 Then
Debug.Print "Non-Array OK"
Else
Debug.Print "Non-Array FAILED!"
End If
'Test Ranks 1 to 100
For TestRank = 1 To 100
TestArr = MakeArray(TestRank)
If Rank(TestArr) = TestRank Then
Debug.Print TestRank & "D OK"
Else
Debug.Print TestRank & "D FAILED!"
End If
Next TestRank
End Sub
'***************************************************************************'
Can the code, comments and test be deemed acceptable, what is there to improve? Is the test okay or have I got the whole unit testing idea wrong?
IsArrInitialized()
and MakeArray(n)
are listed for completeness here. Note that MakeArray(n)
is only used to create test arrays and is private to the array test module.
'***************************************************************************'
Private Function MakeArray( _
RankArr As Integer, _
Optional Value As Variant = 1 _
) As Variant
Dim TestArr As Variant
Dim DummyArr As Variant
Dim i As Integer
If RankArr = 0 Then
MakeArray = Value
ElseIf RankArr = 1 Then
ReDim TestArr(1 To 1)
TestArr(1) = Value
MakeArray = TestArr
Else
ReDim TestArr(1 To 1)
ReDim DummyArr(1 To 1)
DummyArr(1) = Value
For i = 1 To RankArr - 1
DoEvents
TestArr(1) = DummyArr
DummyArr = TestArr
Next i
MakeArray = TestArr
End If
End Function
'***************************************************************************'
'***************************************************************************'
'Determines if a dynamic array has been initialized '
'Parameters: '
' Arr: The array to be processed '
'Returns: '
' True if initialized, False if not '
'***************************************************************************'
Public Function IsArrInitialized(ByRef Arr As Variant) As Boolean
'Declarations **************************************************************'
Dim dum As Long
'***************************************************************************'
On Error GoTo ErrLine
If IsArray(Arr) Then
dum = LBound(Arr)
If dum > UBound(Arr) Then
GoTo ErrLine
Else
IsArrInitialized = True
End If
Else
IsArrInitialized = False
End If
Exit Function
ErrLine:
IsArrInitialized = False
End Function
'***************************************************************************'
2 Answers 2
Regarding the not-listed IsArrInitialized
, after investigating on StackOverflow, I ended up using this:
Private Declare Sub CopyMemory Lib "kernel32" Alias "RtlMoveMemory" _
(pDst As Any, pSrc As Any, ByVal ByteLen As Long)
'returns true if specified array is initialized.
Public Function IsArrayInitialized(arr) As Boolean
Dim memVal As Long
CopyMemory memVal, ByVal VarPtr(arr) + 8, ByVal 4 'get pointer to array
CopyMemory memVal, ByVal memVal, ByVal 4 'see if it points to an address...
IsArrayInitialized = (memVal <> 0) '...if it does, array is intialized
End Function
I would love to see your MakeArray
implementation, but I'll stick to the Rank
function.
Comments
'***************************************************************************'
'Returns the rank of the passed array '
'Parameters: '
' Arr: The array to be processed '
'Returns: '
' The rank of the array '
'***************************************************************************'
I remember being told, in school, that this was a good way of documenting code. However, real-world experience has proven otherwise. These comments only clutter up the code, add to the burden of maintenance and inevitably become stale/obsolete/lies.
What's the need exactly? Say what the thing does. Good, useful comments don't do that - the code itself says "what", good comments say "why". How do you know what the thing does? With proper encapsulation and naming!
If the Rank
function is defined in a code module called Helpers.bas
that contains dozens of unrelated specialized functions like this, you have a problem. If it's defined in a code module called ArrayHelpers.bas
that contains dozens of somewhat related specifalized functions like this, you have a lesser problem, but a problem still: in VBA anything Public
declared in a code module (.bas) is accessible as a "macro" - a Public Function
in Excel VBA could even be used as a cell formula, so naming is very important to avoid clashes with existing/"native" functions.
'Declarations **************************************************************'
Please don't. If you feel the need to "sectionize" the code inside a function, chances are that function is doing too many things.
Dim MaxRank As Byte 'Maximum rank of the elements of the array
These comments shouldn't be needed; it should be clear what MaxRank
is used for, and if it isn't, then the identifier needs a better name, not a comment.
Naming
I think Rank
is a poor name for what the function does. First, I'm tempted to read it as a verb, but you intend it as a noun - nouns are generally classes (/objects), and verbs are for methods, functions and procedures.
Readability
Your function is recursive, and it's not clear what the intent is - arrays have indices, not "ranks" - the multiple assignments of the function's return value are also a hinderance, consider this:
Public Function Rank(ByRef Arr As Variant) As Byte
Dim Result As Byte
Dim MaxRank As Byte
Dim i As Integer
If IsArray(Arr) Then
If IsArrInitialized(Arr) Then
MaxRank = 0
For i = LBound(Arr) To UBound(Arr)
Result = Rank(Arr(i)) + 1 ' recursive call
If Result > MaxRank Then MaxRank = Result
Next i
Result = MaxRank
Else
Result = 0
End If
Else
Result = 0
End If
Rank = Result
End Function
The function's return value being only assigned once, makes it easier to rename it and easier to read and tell the reads from the writes and recursive calls at a glance.
Bug?
Unless you've specified Option Base 1
VBA arrays are 0-based, which means a return value of 0 could be problematic. Typical VBA code would return -1 for an invalid index, so you can tell an invalid index from a zero-based first index.
This, of course, is countered by explicitly declaring the lower bound of your arrays (1 to 1, 1 to 1, 1 to n)
, but that's a rather verbose way of declaring an array.
That said, I'm not 100% clear on exactly what that function is achieving, even after reading your SO post. Is it possible that you don't really need a multidimensional array (1,1,n), but rather a list of objects that encapsulate an array and two other fields? This code could be of interest.
-
\$\begingroup\$ Thank you for the thorough review. I will definitely keep your points in mind when I revise this and write new code. I tend to use too much comments, I know and this is due to the fact that a DBA with little xp in programming will have to maintain, while I'm away for some months. A return value of 0 would actually mean that the variant is actually a single value and not an array. I added the missing functions and also stated my motivation to use such a function. \$\endgroup\$Teeracroptus– Teeracroptus2014年04月04日 08:47:59 +00:00Commented Apr 4, 2014 at 8:47
From above:
you have a lesser problem, but a problem still: in VBA anything Public declared in a code module (.bas) is accessible as a "macro" - a Public Function in Excel VBA could even be used as a cell formula, so naming is very important to avoid clashes with existing/"native" functions.
To keep your internal public subs and functions from appearing in the user macro list, define them with an optional parameter, even though it is never used.
Public Sub YourName(Optional Dummy As Variant = Nothing)
' Your code goes here.
End Sub
-
\$\begingroup\$ Dummy arguments? NO! Terrible practice. There is literally an in-built option for this,
Option Private Module
. \$\endgroup\$Kaz– Kaz2016年08月04日 13:56:23 +00:00Commented Aug 4, 2016 at 13:56