Test for Array vs Range in my TEXTJOIN UDF I use the following code in many of my answers on Stack Overflow, to mimic the new TEXTJOIN function available in Office 365 Excel:

Function TEXTJOIN(delim As String, skipblank As Boolean, arr)
    Dim d As Long
    Dim c As Long
    Dim arr2()
    Dim t As Long, y As Long
    t = -1
    y = -1
    If TypeName(arr) = "Range" Then
        arr2 = arr.Value
    Else
        arr2 = arr
    End If
    On Error Resume Next
    t = UBound(arr2, 2)
    y = UBound(arr2, 1)
    On Error GoTo 0

    If t &gt;= 0 And y &gt;= 0 Then
        For c = LBound(arr2, 1) To UBound(arr2, 1)
            For d = LBound(arr2, 1) To UBound(arr2, 2)
                If arr2(c, d) &lt;&gt; "" Or Not skipblank Then
                    TEXTJOIN = TEXTJOIN &amp; arr2(c, d) &amp; delim
                End If
            Next d
        Next c
    Else
        For c = LBound(arr2) To UBound(arr2)
            If arr2(c) &lt;&gt; "" Or Not skipblank Then
                TEXTJOIN = TEXTJOIN &amp; arr2(c) &amp; delim
            End If
        Next c
    End If
    TEXTJOIN = Left(TEXTJOIN, Len(TEXTJOIN) - Len(delim))
End Function


I have successfully made it so it works with both ranges and arrays, so it works with:

=TEXTJOIN(",",TRUE,A1:B7)


As well as

{=TEXTJOIN(",",TRUE,IF(A1:A7 = "x",B1:B7,""))}


The question is:  Is this the best method to check for array vs. range?  Is there a better method?

As stated this works, nicely with both, but I have to think that I am doing it in a round about manner.
Okay, let's first eliminate the easy stuff Rubberduck points out:


Function is implicitly Public and implicitly returns a Variant - should be a String.
All parameters are implicitly passed ByRef (and semantically should be ByVal).
Parameter arr is implicitly Variant,
Local variables c, d, t, y, arr2 all have terrible, meaningless names (arr, matey! ..I like calling it Pirate Notation ;)
String-returning Left$ function should be used over Variant-returning Left function.
vbNullString should probably be preferred over "" empty string literal.


With default settings Rubberduck will also complain about Dim t As Long, y As Long, because having multiple declarations in a single instruction isn't ideal.

The variables are declared at the top of the function rather than as close as possible to their usage, which makes it hard to see what's used where.



Type-checking isn't type safe:


If TypeName(arr) = "Range" Then
    arr2 = arr.Value
Else
    arr2 = arr
End If



If the Microsoft Word object model is referenced, you can pass a Word.Range object and it will happily take it; same if I made my own Range class and gave it an instance. Use the TypeOf operator to perform compile-time type-safe type checks:

If TypeOf arr Is Excel.Range Then
    'definitely an Excel Range object
Else
    'could be anything
End If


Note, in the Else branch arr could literally be anything - but you're assuming it's an array. You could use the IsArray function to make sure of that, and then you can also assert on the upper bound using a rather ugly helper function, to make sure you're looking at the 2D array you're expecting.

Else
    Debug.Assert IsArray(arr)
    Debug.Assert GetArrayDimSize(arr) = 2
    arr2 = arr
End If


With the array dimension count known/asserted, the On Error Resume Next statement that follows can be removed... and t can be renamed sourceColumns or colCount or whatever, and y can be renamed sourceRows or rowCount or whatever - and arr2 could be renamed to sourceArray:

sourceColumns = UBound(sourceArray, 2)
sourceRows = UBound(sourceArray, 1)


And with that naming scheme I think I'd rename the arr parameter to source.


If t &gt;= 0 And y &gt;= 0 Then



Ah, ok, so that condition is leveraging the fact that VBA would have thrown an error given a 1D array, leaving t = 0. Not very obvious, let's improve that.

But first we need to tweak the assertions - we don't really want a 2D array, we just want any array with at most two dimensions. So... let's be less forgiving given anything other than that:

Dim dimensionCount As Long
If TypeOf(arr) Is Excel.Range Then
    sourceArray = source.Value
    dimensionCount = 2 'Range.Value is always a 2D array
Else
    Dim isValidArray As Boolean
    isValidArray = IsArray(source)
    If isValidArray Then dimensionCount = GetArrayDimSize(source)
    isValidArray = dimensionCount &lt;&gt; 0 And dimensionCount &lt;= 2
    If Not isValidArray Then Err.Raise 5, "TEXTJOIN", "Expected: 1D or 2D array"
End If


And now that If statement can be much more explicit about what's going on and why:

If dimensionCount = 2 Then
    'handle 2D array
Else
    Debug.Assert dimensionCount = 1
    'handle 1D array
End If


So, c iterates dimension 1 / rows, d iterates dimension 2 / columns.

Now all these concatenations are inherently slow. I realize that's "just a worksheet function" and you'd like to share it as a single, simple, cohesive and focused little piece of code... but given thousands of iterations, VBA's string-handling will start becoming the performance bottleneck of the function.

There's a lightning-fast StringBuilder class right here on this site that you can use to address that.

As a bonus, using a StringBuilder makes the function assign to its return value only once - as opposed to what you have now, which is treating the return-value-identifier as if it were a local variable (which it technically is, ...I just don't like doing that).

Not sure why the d / column loop is inconsistent here:


For d = LBound(arr2, 1) To UBound(arr2, 2)



You're iterating the 2nd dimension, the LBound should be off the 2nd dimension too (yes, it should be the very same as that of the 1st dimension). Also, the upper bounds of both dimensions are already known and stored in local variables:

For currentRow = LBound(sourceArray, 1) To sourceRows
    For currentColumn = LBound(sourceArray, 2) To sourceColumns


This code will throw an error (by design?) if the array contains an Error value:


If arr2(c, d) &lt;&gt; "" Or Not skipblank Then



You could have a parameter that helps you decide how to treat errors - perhaps an Enum could be used:

Public Enum TEXTJOIN_ErrorValues
    ThrowOnError
    SkipError
    IncludeErrorText
End Enum


ThrowOnError would be the current/default behavior; SkipError would treat errors as blanks, and IncludeErrorText would include the e.g. #N/A error text into the result.

Except, once you have an Variant/Error value and not a Range, it's pretty much impossible to get the text back (unless you want to map CVErr(xlErrWhatever) values to a corresponding string... probably not worth it) - so scratch that enum value, and the behavior becomes either throw on error, or skip error values. And that's entirely possible, and quite easy to do - an additional optional Boolean parameter could be helpful for that.

Dim sb As StringBuilder
Set sb = New StringBuilder
'...

For currentRow = LBound(sourceArray, 1) To sourceRows
    For currentColumn = LBound(sourceArray, 2) To sourceColumns
        If Not IsError(sourceArray(currentRow, currentColumn)) Then
            If sourceArray(currentRow, currentColumn) &lt;&gt; vbNullString Or Not skipBlank Then
                sb.Append sourceArray(currentRow, currentColumn)
                sb.Append delim
            End If
        ElseIf Not skipErrors Then
            sb.Append delim
        End If
    Next
Next

TEXTJOIN = sb.ToString


Now, that's the 2D loop.. the 1D loop does essentially the same thing.. and that's annoying. I'd extract a method for that.

Private Sub ProcessValue(ByVal value As Variant, ByVal sb As StringBuilder, ByVal delim As String, ByVal skipBlanks As Boolean, ByVal skipErrors As Boolean)
    If Not IsError(value) Then
        If CStr(value) &lt;&gt; vbNullString Or Not skipBlanks Then
            sb.Append CStr(value)
            sb.Append delim
        End If
    ElseIf Not skipErrors Then
        sb.Append delim
    End If
End Sub


That turns the logic into:

If dimensionCount = 2 Then
    For currentRow = LBound(sourceArray, 1) To sourceRows
        For currentColumn = LBound(sourceArray, 2) To sourceColumns
            ProcessValue sourceArray(currentRow, currentColumn), sb, delim, skipBlanks, skipErrors
        Next
    Next
Else
    Debug.Assert dimensionCount = 1
    For currentRow = LBound(sourceArray, 1) To sourceRows
        ProcessValue sourceArray(currentRow), sb, delim, skipBlanks, skipErrors
    Next
End If

TEXTJOIN = sb.ToString




The order of the parameters strikes me as unintuitive - might be by design to match Microsoft's function, by I would have made the source array/range the first parameter, followed by an optional delimiter, followed by an optional flag to skip blanks (followed by an optional flag to skip errors).