6
\$\begingroup\$

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
'***************************************************************************'
asked Apr 3, 2014 at 10:55
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

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.

answered Apr 3, 2014 at 16:03
\$\endgroup\$
1
  • \$\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\$ Commented Apr 4, 2014 at 8:47
3
\$\begingroup\$

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
answered Jan 20, 2016 at 11:46
\$\endgroup\$
1
  • \$\begingroup\$ Dummy arguments? NO! Terrible practice. There is literally an in-built option for this, Option Private Module. \$\endgroup\$ Commented Aug 4, 2016 at 13:56

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.