Define a range based on passed arguments

Question

TL;DR I'm self teaching VBA, learning about functions and passing arguments from a subroutine into the function and want to know if my very basic function to define a range is optimal and follows correct referencing/standards/practices.

I'm teaching myself VBA (mostly in MS Excel) as a starting point into programming in my spare time (what little of it I have) with the goal of a career change and am now learning how to utilise functions.

Below is a function I have written to define a range based on the variables passed as arguments. As you will see the worksheet, first row and first column are required arguments and the second row and second column are optional.

The function will assign the value of the first row (or column) to the second row (or column) if the optional argument is omitted.

Function defineRange(mySheet As Object, startRow As Long, firstColumn As Long, Optional endRow As Long, Optional secondColumn As Long) As Object

'will create a single row range
If endRow = 0 Then
    endRow = startRow
End If
'will create a single column range
If secondColumn = 0 Then
    secondColumn = firstColumn
End If

'Define the A1 reference for the range with cells.address parameter using arguments as the row and column index.
Set defineRange = mySheet.Range(Cells(startRow, firstColumn).Address, Cells(endRow, secondColumn).Address)

End Function

As an example I'd call it from a subroutine as follows:

Sub test()

Dim myRange As Object

Set myRange = defineRange(ThisWorkbook.Sheets(1), 5, 2, 10, 4)

myRange.Select

End Sub

The result is Range("B5:D10") is selected on sheet1 of the workbook.

Is the function code optimal? Have best practices been followed?

Answer 1

First, kudos for teaching yourself VBA - I was once in your shoes and how I wish I could have had Code Review to help me back then!

Let's start with the function's signature:

Function defineRange(mySheet As Object, startRow As Long, firstColumn As Long, Optional endRow As Long, Optional secondColumn As Long) As Object

It's not clear whether the function's implicit public accessibility is intended or not. If it means to be accessible from outside the module it's declared in, then it should be explicitly Public. Otherwise, it should be Private - a private procedure/function/method can only be invoked from within the module it belongs to. I'm thinking the intent is for it to be Public here.

You probably noticed pretty much everything in VBA type libraries is PascalCase. From Range to Worksheet.Name - best practice is to "blend in" and write code that adheres to the naming standard. Hence, DefineRange would be better. I like how the name begins with a verb (as it should!), however "define" doesn't strike me as the best choice in this case. The function is getting, or rather acquiring a Range object. The Excel object model already provides an API for this, so reinventing-the-wheel is appropriate here :)

The parameters are all implicitly passed by reference (ByRef), which is an unfortunate default. Would this be expected/desired behavior?

Dim firstRow As Long
firstRow = 12

Dim lastRow as Long
lastRow = 0

Dim target As Range
Set target = defineRange(Sheet1, firstRow, 1, lastRow)

Debug.Print lastRow ' prints 12, not 0. is that expected?

By passing the parameters ByVal, instead of passing a pointer ("reference") to the value, you essentially pass a copy of that value, and the calling code's variables can't get modified by the function. In VB.NET and many other languages, the implicit default is to pass arguments by value: it's somewhat rare that a parameter needs to be passed by reference.

mySheet is declared as an Object, which means the function will happily take a Collection, some UserForm1, any Range, or even MyClass for this mySheet parameter... and then the mySheet.Range call will fail at run-time with error 438 "can't find property or method" (unless there's a method named Range on that UserForm1 or MyClass object, of course). By declaring it As Object, you made every member calls against that object late-bound, meaning the compiler can't help you, and will happily let you try to invoke mySheet.Rnge.

It's usually best to avoid deferring failures to run-time, and fail at compile-time instead, whenever possible. We do this with early binding, by declaring objects using a specialized interface we know we can work with - in this case, Excel.Worksheet, or just Worksheet. We can do this, because when VBA is hosted in Excel, the VBA project is guaranteed to have a reference to the Excel type library. If we were hosted in Word or PowerPoint, we would have to explicitly add that reference (through tools/references), or work late-bound and keep it As Object.

The parameter names are meant to be pairs, but they're inconsistent:

• startRow, endRow
• firstColumn, secondColumn

"Start/End" was a better, clearer idea than "First/Second" - I'd go and rename the parameters to have startColumn and endColumn, matching startRow and endRow.

Kudos for declaring an explicit return type - all Function procedures return something, whether you declare a return type or not. Then again, it would be better to return a Range rather than an Object.

So, that covers the function's signature :)

---

There are a number of possible bugs and edge cases that should be handled. The first thing any function should do, is validate its inputs.

VBA doesn't have unsigned integer types, so a Long could very well be a negative number, or it could be zero -- but Excel's object model isn't going to like you trying to get the cell at row 0 and column -728.

There are several ways to deal with this. The simplest is to use Debug.Assert at the very top of the function, i.e. halt program execution if assumptions aren't validated:

'Debug.Assert TypeOf mySheet Is Excel.Worksheet
Debug.Assert startRow > 0
Debug.Assert firstColumn > 0

Another way is to have a guard clause, again at the very top of the function, that explicitly throws an error given invalid arguments:

If startRow > 0 Then Err.Raise 5, "defineRange", "Argument 'startRow' must be greater than zero."
If firstColumn > 0 Then Err.Raise 5, "defineRange", "Argument 'firstColumn' must be greater than zero."

One advantage of using guard clauses, is that if a function is given arguments it cannot possibly work with, then we fail early. This makes it easier to debug if something goes wrong later: instead of dealing with a cryptic and rather useless "Method 'Range' of class 'Worksheet' failed" error, the code that tried to invoke our function now knows exactly what went wrong, and how to fix it.

The return value assignment will fail if mySheet isn't the ActiveSheet:

mySheet.Range(Cells(startRow, firstColumn).Address, Cells(endRow, secondColumn).Address)

That's because the unqualified Cells calls are context-dependent.

If the function is written in a worksheet module's code-behind, then it's implicitly Me.Cells.

If the function is written anywhere else, then it's implicitly [_Global].Cells, which ultimately resolves to ActiveSheet.Cells. That isn't a problem if mySheet is active, but then if it isn't...

Sheet1.Range(Sheet2.Cells(...), Sheet2.Cells(...))

That's guaranteed to throw the dreaded run-time error 1004 "application-defined error", because only Schrödinger's Range can belong to two worksheets at the same time.

You can fix this with a With block:

With mySheet
    Set defineRange = .Range(.Cells(startRow, firstColumn).Address, .Cells(endRow, secondColumn).Address)
End With

Note the . dereferencing operators qualifying the Cells member calls with the object reference held by the With block. It's equivalent to this:

Set defineRange = mySheet.Range(mySheet.Cells(startRow, firstColumn).Address, mySheet.Cells(endRow, secondColumn).Address)

That said, you don't need to work off the .Address string: Range is more than happy to work with the Range references returned by Cells(...):

Set defineRange = mySheet.Range(mySheet.Cells(startRow, firstColumn), mySheet.Cells(endRow, secondColumn))

While that works, I find it quite a mouthful, and as shown above, it has too many reasons to fail to my taste - I'd probably split it up:

Dim startCell As Range
Set startCell = mySheet.Cells(startRow, firstColumn)

Dim endCell As Range
Set endCell = mySheet.Cells(endRow, secondColumn)

Making the return assignment rather trivial:

Set defineRange = mySheet.Range(startCell, endCell)