1

I've tried searching the internet for a definitive answer to this two-part scenario but couldn't find anything conclusive. I've been writing VBA procedures for sometime now in both Access and Excel and while trying to streamline some code I came across something of a dilemma

The first part is about using functions to return objects. The example below is generally what I've seen on the web for a function to return an ADODB.Recordset (I've simplified the code so no error handling etc.).

Public Function CreateADORecordset(SQL As String, Connection As ADODB.Connection) As ADODB.Recordset

Dim rst As ADODB.Recordset
Set rst = New ADODB.Recordset
Call rst.Open(SQL, Connection)
Set CreateADORecordset = rst

End Function

The first part of the question is, why do I need a variable called rst when I could rewrite the function as this:

Public Function CreateADORecordset(SQL As String, Connection As ADODB.Connection) As ADODB.Recordset

Set CreateADORecordset = New ADODB.Recordset
Call CreateADORecordset.Open(SQL, Connection)

End Function

Is there anything fundamentally wrong with the above rewrite of the function? As the function returns an ADO recordset, why declare a variable separately?

I can take this a step further:

Public Function CreateADOConnection(ConnectionString As String) As ADODB.Connection

Set CreateADOConnection = New ADODB.Connection
Call CreateADOConnection.Open(ConnectionString)

End Function

Public Function CreateADORecordset(SQL As String, ConnectionString As String) As ADODB.Recordset

Set CreateADORecordset = New ADODB.Recordset
Call CreateADORecordset.Open(SQL, CreateADOConnection(ConnectionString))

End Function

Again, is this a particularly bad thing to do by using function return objects over declaring objects within the procedures via Dim?

In the grand scheme of things, I've been writing VBA code to transfer the contents of a recordset via GetRows into an Excel range. The function declaration line is:

Public Sub TransferRecordsetArray(GetRows As Variant, Destination As Range)

As TransferRecordsetArray works correctly, I've not included the code.

The dilemma I'm in now is in this scenario, I've reached a point where I don't need to declare any variables for the code to run correctly and I'm unsure how much of a good or bad thing this in terms of functions returning objects, scope and variables, etc.

In order to run the code correctly, I only need one of two lines without variables:

Call TransferRecordsetArray(CreateADOConnection(ConnectionString).Execute(SQL).GetRows, Target)

or

Call TransferRecordsetArray(CreateADORecordset(SQL, CreateADOConnection(ConnectionString)).GetRows, Target)

Any advice/recommendations on this way of writing VBA code would be greatly appreciated. I have used the task manager to keep an eye on memory usage on both methods and it doesn't seem to greatly differ and, it appears that VBA destroys the function return objects after a while, despite them not being explicitly destroyed by setting them to Nothing.

Many thanks.

DS94
  • 13
  • 2
  • `Call CreateADORecordset.Open(SQL, Connection)` - you can drop the `Call` and the parentheses: `CreateADORecordset.Open SQL, Connection`. – BigBen Oct 28 '20 at 14:00
  • @freeflow "VBA can get stuck" not really. That's a static error. Either the code compiles, or it doesn't. VBA won't get stuck. – Tomalak Oct 28 '20 at 15:38

1 Answers1

0

The first part of the question is, why do I need a variable called rst when I could rewrite the function as this

Public Function CreateADORecordset(SQL As String, Connection As ADODB.Connection) As ADODB.Recordset

Set CreateADORecordset = New ADODB.Recordset
Call CreateADORecordset.Open(SQL, Connection)

End Function

You don't need a separate variable. Your code is perfectly fine.

I can take this a step further:

Public Function CreateADOConnection(ConnectionString As String) As ADODB.Connection

Set CreateADOConnection = New ADODB.Connection
Call CreateADOConnection.Open(ConnectionString)

End Function

Public Function CreateADORecordset(SQL As String, ConnectionString As String) As ADODB.Recordset

Set CreateADORecordset = New ADODB.Recordset
Call CreateADORecordset.Open(SQL, CreateADOConnection(ConnectionString))

End Function

Yes, absolutely. Nothing wrong with that.

I've reached a point where I don't need to declare any variables for the code to run correctly

Congratulations, keep it up. :)

Further reading: Is there a need to set Objects to Nothing

Tomalak
  • 332,285
  • 67
  • 532
  • 628
  • In general, I agree with what you said. With the last and final scenario, my concerns would be readability and more importantly, error handling. What if a connection cannot be made? What if data retrieval fails? So yes, the syntax is valid but stringing together method calls assumes too much. – Brian M Stafford Oct 28 '20 at 14:11
  • @BrianMStafford The OP stated that they took out error handling for the purpose of the question. When `CreateADOConnection` fails, it can handle the error internally just as before. When downstream code *calling* `CreateADOConnection` fails, the same applies. It's a question of preference (and necessity, or lack thereof) whether "doing it all on one line" is an option, but that's not the scope of the question IMO. – Tomalak Oct 28 '20 at 14:28
  • @BrianMStafford in response to the readability issue, I have a rule that no line of code should exceed 95 characters (this seems to be the maximum size that A4 paper can accommodate when printing from within the VBE). If a line does get too long, there's always the option of a line continuation. – DS94 Oct 29 '20 at 10:05