3
\$\begingroup\$

My VBA has become really rusty but I need it for now for some data downloading.

This routine works but I'm certain that over the last 10 (at least) years something quicker/less code has surfaced to do the same thing. Any help appreciated. Please forgive my non-standard casing for VBA.

Public Function GetSymbols() As Variant

Dim ws As Worksheet
Set ws = Sheets(SymbolSheetName)

Dim tempRange, cCell As Range
Set tempRange = ws.Range(SymbolStartCell)
Set tempRange = Range(tempRange, tempRange.End(xlDown))

Dim symbolsArray() As String
Dim cntr As Integer
cntr = 0

For Each cCell In tempRange
    ReDim Preserve symbolsArray(cntr)
    symbolsArray(cntr) = cCell.Value
    cntr = cntr + 1
Next cCell
GetSymbols = symbolsArray
End Function
\$\endgroup\$
0

1 Answer 1

7
\$\begingroup\$

As Integer would be a problem with more than 32,767 cells - better to use As Long here.

Set tempRange in two consecutive assignments is suspicious, and the unqualified Range member call in the second expression will throw error 1004 whenever ws isn't also the ActiveSheet (unless that code is written in the code-behind for that particular worksheet... in which case both SymbolSheetName and ws are redundant; simply use the code name to refer to any worksheet that exists in ThisWorkbook at compile-time).

Dim tempRange, cCell As Range is declaring cCells as a Range, and leaves tempRange as an implicit Variant.

Assigning a Variant cell's value to a String array element without validating the value would throw type mismatch errors if the cells contain any #VALUE! (or any other type of) worksheet errors; you should use If Not IsError(cCell.Value) to first validate whether the conversion can happen in the first place.

ReDim Preserve inside a loop is the killer; you're looking at a Range, and you know in advance how many cells you're looking at - you could size the array once and that would already be an improvement, but...

...you never need a loop to get a Variant array out of every cell in a Range:

Dim values As Variant
values = sourceRange.Value ' boom, values now contains a 2D variant array

If you want a single-dimension array and the range is small enough, you can use Application.Transpose to get it:

Dim values As Variant
values = Application.Transpose(sourceRange.Value)

Note that the way you get the cells you're looking for could lead to unexpected results if there are any empty cells in the range: by starting at the top and going xlDown from the first cell, your last cell is going to be at the first empty one. Usually we start at the bottom of the sheet instead, and go xlUp, making the last cell the first non-empty cell starting from the bottom, thus making fewer assumptions about the shape and content of the data - something like this:

Dim lastRow As Long
lastRow = sheet.Range("A" & sheet.Rows.Count).End(xlUp).Row

Lastly, in the last decade Rubberduck appeared (a free & open-source VBIDE add-in I made with a couple of other VBA nerds) and can analyze your code and warn you about several things I noted in this answer.

\$\endgroup\$
4
  • \$\begingroup\$ Thanks. I use xlDown because the dataloading tool I am using will only produce usable rows. I wrote hundreds of macros using common types like Dim a,b,c as Range, never knew the first ones would turn Variant, somewhere I am sure I read you could do that that way. I wont get string errors, the range is a one column array of strings \$\endgroup\$
    – dinotom
    Commented Feb 26, 2022 at 16:24
  • \$\begingroup\$ ...why does the Transpose method create an array where the first index is 1 and not 0? \$\endgroup\$
    – dinotom
    Commented Feb 26, 2022 at 16:34
  • 2
    \$\begingroup\$ @dinotom the array comes from a Range, there's no row or column 0 in Excel, so Range arrays are 1-based so the indexes line up with cell rows/columns. \$\endgroup\$ Commented Feb 26, 2022 at 17:15
  • 1
    \$\begingroup\$ @dinotom you can declare multiple on one line like that in vb.net which is syntactically very similar and leads to confusing Google results, but not in VBA \$\endgroup\$
    – Greedo
    Commented Feb 26, 2022 at 23:00

Not the answer you're looking for? Browse other questions tagged or ask your own question.