1
\$\begingroup\$

I'm iterating through a list of possible values that reside in a named range on a worksheet. Since accessing ranges is slow, I wanted an easy way to convert them to an array and came up with the function below that does so. Is there a better way of setting my code up?

Public Function ConvertRangeIntoArray(ByVal convertRange As Range) As String()
    Dim temporaryVariant As Variant
    Dim is2DArray As Boolean
    Select Case True
        Case convertRange.Rows.Count = 1
            temporaryVariant = Application.WorksheetFunction.Transpose(Application.WorksheetFunction.Transpose(convertRange))
        Case convertRange.columns.Count = 1
            temporaryVariant = Application.WorksheetFunction.Transpose(convertRange)
        Case Else
            temporaryVariant = convertRange
            is2DArray = True
    End Select

    Dim returnValue() As String
    Dim rw As Long
    Dim cl As Long
    If is2DArray Then
        ReDim returnValue(LBound(temporaryVariant) To UBound(temporaryVariant), LBound(temporaryVariant, 2) To UBound(temporaryVariant, 2))
        For rw = LBound(temporaryVariant) To UBound(temporaryVariant)
            For cl = LBound(temporaryVariant, 2) To UBound(temporaryVariant, 2)
                returnValue(rw, cl) = CStr(temporaryVariant(rw, cl))
            Next
        Next
    Else
        ReDim returnValue(LBound(temporaryVariant) To UBound(temporaryVariant))
        For cl = LBound(temporaryVariant) To UBound(temporaryVariant)
            returnValue(cl) = CStr(temporaryVariant(cl))
        Next
    End If

    ConvertRangeIntoArray = returnValue
End Function
\$\endgroup\$
  • \$\begingroup\$ What's the point of the double WorksheetFunction.Transpose? \$\endgroup\$ – Snowbody Jan 12 '18 at 21:08
2
\$\begingroup\$

Kudos for explicit Public and the ByVal parameter. I wouldn't make it return a String() array though.

You can't know that every value in the source Range can be converted to a String.

returnValue(cl) = CStr(temporaryVariant(cl))

This instruction will make your function fail if the convertRange contains any Error value. You need to first verify whether IsError(temporaryVariant(cl)) before you can attempt any safe type conversion.

But then, what would you do with a value that can't be converted? Remove it from the results? Return an empty string?

What if the range is all numeric values, that the calling code needs to add up; there's some serious overhead going on with iterating both dimensions of a 2D array to convert everything to a String, and then doing the same thing again to convert strings back to Double for proper calculations.

IMO that's one of the cases where you're better off just returning a Variant. That significantly cuts down the implementation:

Public Function ToArray(ByVal target As Range) As Variant
    Select Case True
        Case target.Rows.Count = 1
            'horizontal 1D range
            ToArray = Application.WorksheetFunction.Transpose(Application.WorksheetFunction.Transpose(target)).Value
        Case target.columns.Count = 1
            'vertical 1D range
            ToArray = Application.WorksheetFunction.Transpose(target).Value
        Case Else
            '2D array: let Excel to the conversion itself
            ToArray = target.Value
    End Select
End Function

Note I renamed the function to a more straightforward ToArray which lets the argument speak for itself, and renamed convertRange to the more idiomatic (it comes up a lot in worksheet event handlers) target (it's also a noun, which is more appropriate than something that looks like a verb for naming a variable).

The caller now has access to the actual types - with everything it means: if a value's type IsError, then the caller needs to deal with it accordingly. If a value IsNumeric, then the caller can perform arithmetic operations on it without incurring implicit type conversions (aside from unwrapping the value from the Variant). If the value IsDate, then the caller can work with dates without ever having to deal with how VBA might be representing that date value in a String.

And performance-wise, it's now as fast as it's going to get (pretty much instant), regardless of the size of the returned array.

I'm not too fond of Select Case True. Case conditions are still going to be evaluated sequentially until one matches, so at the end of the day it's not much different from a more conventional construct:

Public Function ToArray(ByVal target As Range) As Variant
    If target.Rows.Count = 1 Then
        'horizontal 1D range
        ToArray = Application.WorksheetFunction.Transpose(Application.WorksheetFunction.Transpose(target.Value))
    ElseIf target.columns.Count = 1 Then
        'vertical 1D range
        ToArray = Application.WorksheetFunction.Transpose(target.Value)
    Else
        '2D array: let Excel to the conversion itself
        ToArray = target.Value
    End If
End Function

...which removes an indentation level, and puts the conditions where we'd normally expect them. We're also explicitly assigning to the .Value of the range, transposed or not, instead of implicitly pulling the ranges' default member.

\$\endgroup\$
  • \$\begingroup\$ Originally it was set up as variant but second guessed myself, I changed it to string since all occurrences will only ever be text. Is there any benefit, aside from less typing of using a Select Case over If ... Else if? \$\endgroup\$ – IvenBach Apr 25 '17 at 19:42
  • \$\begingroup\$ I rarely use Select Case for anything other than dealing with enum values. I guess it depends ;-) ..as for the string conversions, if the caller is going to be iterating the array then it's best to let the caller do the string conversions; that way you only iterate it once. \$\endgroup\$ – Mathieu Guindon Apr 25 '17 at 19:45
  • 2
    \$\begingroup\$ Shouldn't we be doing Toarray = target.value? Since ToArray is returning a variant, it could return target as a range. This wouldn't be the desired effect (unless I am mistaken about the intent). To clarify, I know the default member of Range is Value but I prefer to rely on explicit assignments and not implicit. I am assuming that we can avoid ToArray becoming the Range by not using the Set keyword, but would using .Value not be a better approach anyways? \$\endgroup\$ – Brandon Barney Apr 26 '17 at 11:44
  • \$\begingroup\$ @BrandonBarney that is absolutely correct! \$\endgroup\$ – Mathieu Guindon Apr 26 '17 at 12:03
  • \$\begingroup\$ @Mat'sMug, you have a minor error in your code. It is ElseIf instead of Else If, right. \$\endgroup\$ – Stefan Pinnow Jan 9 '18 at 10:30
0
\$\begingroup\$

Take a look at http://www.cpearson.com/excel/ArraysAndRanges.aspx

If you declare your array variable as

Dim temporaryVariant() As Variant

it will automatically pick up the cells as a 2-d array, without needing any funny business to avoid picking up the Range object.

That should make it a lot easier to make your String() later.

You should also cache the LBound() and UBound() results; it's faster (and less code)

\$\endgroup\$

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.