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)
