HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Concatenate disjoint Excel ranges to use as a single range in a formula

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
excelrangeconcatenaterangessingledisjointuseformula

Problem

I need to vertically concatenate two disjoint ranges in VBA to be passed to an Excel function (in this case the LINEST function). I hate working with arrays in VBA but I managed to get it working using the following code:

Option Base 1

Function VERTCAT(R1 As Range, R2 As Range) As Variant

    Dim ncols As Integer
    ncols = R1.Columns.Count
    Dim nrows1 As Integer
    nrows1 = R1.Rows.Count
    Dim nrows2 As Integer
    nrows2 = R2.Rows.Count

    Dim arr1 As Variant
    arr1 = R1
    Dim arr2 As Variant
    arr2 = R2

    Dim arr3 As Variant
    ReDim arr3(nrows1 + nrows2, ncols)

    For Col = 1 To ncols
        For Row = 1 To nrows1
            arr3(Row, Col) = arr1(Row, Col)
        Next Row
        For Row = 1 To nrows2
            arr3(Row + nrows1, Col) = arr2(Row, Col)
        Next Row
    Next Col

    VERTCAT = arr3

End Function


I'm not worried about adding basic error checking, like making certain that the two arrays have the same number of columns or adding features. I am hoping that someone can show me what the idiomatic VBA way is to have written this function? Could I have avoided some of the helper variables (like arr3 for example?)? Could I have avoided the loops?

Solution

To answer your question, no. There is no "idiomatic" way to combine multiple arrays together without creating a bigger array (or extending one of the originals) and manually copying in the array contents.

Anyway, while I'm here,

Miscellaneous Points

I don't know if you just left it out, but if it's not there, Option Explicit should be at the top of every single code module everywhere. IF you don't have it enabled, go to Tools --> Options --> Require Variable Declaration.

Your function is implicitly public because you didn't scope it. Explicitly scope everything. Assuming you want to use it in other modules, it should be Public Function VERTCAT() as Variant.

Integer is outdated. Unless you're in a position where 4 bytes of memory has serious performance impacts, use Long instead. Integer will fail if you give it a number greater than about 33,000, that's a number that could reasonably occur when dealing with big spreadsheets. Long will handle numbers up to 2.1 Billion and is also (marginally) faster.

Variable Naming

First off, VERTCAT violates several good practices.

Standard VBA convention for function/procedure names is PascalCase. VERTCAT is also distinctly unclear and ambiguous. Names should always be clear, concicse and most importantly, unambiguous.

If I see VERTCAT I'm going to think "What does this do? Vertically... catenate?". Okay, but catenate what? Strings? Lists? Ranges? Arrays? VerticallyConcatenateRanges really isn't very long, and is absolutely clear on what it takes as parameters, and what it does to them. Depending on your style, you might consider ArrayFromConcatenatedRanges to be even better.

If in doubt, always err on the side of more verbose.

VBA conventions you should be aware of:

Function/Procedure names are PascalCase.

Local variable names are camelCase so nCols instead of ncols and col instead of Col.

N.B. if Col is capitalised because it's a module-level variable, that's an even worse offence (unnecessarily broad scope) than bad naming.

Module/Global variable names are PascalCase so if I see ReferenceVariable in the middle of a procedure, I know to look for it outside of that procedure's scope.

And now for your variables

R1 and R2 are terrible variable names. Variables should always sound like what they are. I suggest topRange,bottomRange. Suddenly, it's clear that they are ranges, and that the first one is the one that's going to end up on top of the final concatenation.

Similarly, ncols,nrows1,nrows2 are not great. It should be nCols, nRows1, nRows2 anyway, and then numColumns,rowsInTopRange,rowsInBottomRange still sound like counts of rows/columns, but it's much clearer what they are/should be. Similarly, arr1 and arr2 could do with the same treatment.

This is your code, with better naming, and it is so much easier for someone to read/follow:

Option Explicit
Option Base 1

Public Function VerticallyConcatenateRanges(topRange As Range, bottomRange As Range) As Variant

    Dim row As Long, col As Long

    Dim numColumns As Long
        numColumns = topRange.Columns.Count

    Dim rowsInTopRange As Long
        rowsInTopRange = topRange.Rows.Count

    Dim rowsInBottomRange As Long
        rowsInBottomRange = bottomRange.Rows.Count

    Dim totalRows As Long
        totalRows = rowsInTopRange + rowsInBottomRange

    Dim arrTopRange As Variant
        arrTopRange = topRange

    Dim arrBottomRange As Variant
        arrBottomRange = bottomRange

    Dim concatenatedArray As Variant
    ReDim concatenatedArray(1 To totalRows, 1 To numColumns)

    Dim ix As Long

        For col = 1 To numColumns

            For row = 1 To rowsInTopRange
                concatenatedArray(row, col) = arrTopRange(row, col)
            Next row

            For row = 1 To rowsInBottomRange
                ix = row + rowsInTopRange
                concatenatedArray(ix, col) = arrBottomRange(row, col)
            Next row

        Next col

        VerticallyConcatenateRanges = concatenatedArray

End Function


And, crucially, if you took any of those lines out of context and gave them to someone, that person would be able to figure out very easily what the variables were and what the code was doing.

Code Snippets

Option Explicit
Option Base 1

Public Function VerticallyConcatenateRanges(topRange As Range, bottomRange As Range) As Variant

    Dim row As Long, col As Long

    Dim numColumns As Long
        numColumns = topRange.Columns.Count

    Dim rowsInTopRange As Long
        rowsInTopRange = topRange.Rows.Count

    Dim rowsInBottomRange As Long
        rowsInBottomRange = bottomRange.Rows.Count

    Dim totalRows As Long
        totalRows = rowsInTopRange + rowsInBottomRange

    Dim arrTopRange As Variant
        arrTopRange = topRange

    Dim arrBottomRange As Variant
        arrBottomRange = bottomRange

    Dim concatenatedArray As Variant
    ReDim concatenatedArray(1 To totalRows, 1 To numColumns)

    Dim ix As Long

        For col = 1 To numColumns

            For row = 1 To rowsInTopRange
                concatenatedArray(row, col) = arrTopRange(row, col)
            Next row

            For row = 1 To rowsInBottomRange
                ix = row + rowsInTopRange
                concatenatedArray(ix, col) = arrBottomRange(row, col)
            Next row

        Next col

        VerticallyConcatenateRanges = concatenatedArray

End Function

Context

StackExchange Code Review Q#110014, answer score: 7

Revisions (0)

No revisions yet.