VBA-Sync code feedback

Question

I was hoping to get some input on a class module I'm designing to abstract away the boilerplate of utilizing asynchronous queries in VBA. cQueryable supports both synchronous and asynchronous queries. So you could do something like call a package to populate temp tables. This would be done synchronously because you'd want this to be completed before you executed your select queries. After, you would then execute select queries on each of the temp tables asynchronously.

This code really just abstracts away a lot of the functionality in the ADODB library. I tried to name my properties and methods similarly to what the objects in that library use where possible. My connectionString property is named similarly to the same one in the ADODB.Connection object. And my CreateParam method is named similarly to the createParameter method of the ADODB.Command object.

A few of the new procedures I've introduced are the sql property. This holds the sql query to be executed (this maps to commandtext in the command object). Another is ProcedureAfterQuery. This is to hold the name procedure to be called by the connection object after it raises an event when the query completes. Others are SyncExecute and AsyncExecute which should describe what they do in their names.

One thing to note about these two is that SyncExecute is a function whereas AsyncExecute is a subroutine. I wanted SyncExecute to return a recordset when it completes. But for AsyncExecute, I wanted it to be a sub I didn't want to imply that it returned anything. I use similar (but different) code to do this. So I think I violate the DRY principle. I could consolidate these two to call one shared subroutine procedure. That shared procedure would then be more complicated, but the code would at least be shared. I don't have a preference one way or another.

Although CreateParam is similar to the CreateParameter method of the command object, there's two differences. One is that the order of the arguments is different. This is mainly because the size and direction parameters are listed as optional parameters with default values. Their default values can just be used when the value is numeric, but size must be specified if the value is a string. So in certain situations, size is optional, whereas in others it's required. And the query will fail if it isn't provided.

Other things I didn't consider (or test) is that I've read that ADODB can be used essentially anywhere a driver can be provided. So this could be used on Excel workbooks, perhaps text files, and other sources rather than just databases. So perhaps the synchronous and asynchronous queries would work there as well. But that's not what I set out to design or test.

I'd appreciate constructive criticism.

VERSION 1.0 CLASS
BEGIN
  MultiUse = -1  'True
END
Attribute VB_Name = "cQueryable"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = False
Attribute VB_Exposed = False
Option Explicit

'Requires a refernce to the Microsoft ActiveX Data Objects 6.1 Library (or equivalent)

Private WithEvents mASyncConn As ADODB.Connection
Attribute mASyncConn.VB_VarHelpID = -1
Private mSyncConn As ADODB.Connection
Private mConn As ADODB.Connection
Private mComm As ADODB.Command
Private mSql As String
Private mProcedureAfterQuery As String
Private mAsync As Boolean
Private mConnectionString As String

Private Const mSyncExecute As Long = -1

Private Sub Class_Initialize()
    Set mComm = New ADODB.Command
    Set mConn = New ADODB.Connection
End Sub

Public Property Let Sql(value As String)
    mSql = value
End Property

Public Property Get Sql() As String
    Sql = mSql
End Property

Public Property Let ConnectionString(value As String)
    mConnectionString = value
End Property

Public Property Get ConnectionString() As String
    ConnectionString = mConnectionString
End Property

Public Property Let procedureAfterQuery(value As String)
    mProcedureAfterQuery = value
End Property

Public Property Get procedureAfterQuery() As String
    procedureAfterQuery = mProcedureAfterQuery
End Property

Public Sub createParam(pName As String, pType As DataTypeEnum, pValue As Variant, Optional pDirection As ParameterDirectionEnum = adParamInput, Optional pSize As Long = 0)
    Dim pm As ADODB.Parameter
    With mComm
       Set pm = .CreateParameter(name:=pName, Type:=pType, direction:=pDirection, value:=pValue, size:=pSize)
       .Parameters.Append pm
    End With
End Sub

Public Function SyncExecute()
    Set mSyncConn = mConn
    If connectionSuccessful Then
        With mComm
            .CommandText = mSql
            Set .ActiveConnection = mSyncConn
            Set SyncExecute = .execute(Options:=mSyncExecute)
        End With
    End If
End Function

Public Sub AsyncExecute()
    Set mASyncConn = mConn
    If connectionSuccessful Then
        With mComm
            .CommandText = mSql
            Set .ActiveConnection = mASyncConn
            .execute Options:=adAsyncExecute
        End With
    End If
End Sub

Private Function connectionSuccessful() As Boolean
    If mConn.State = adStateClosed Then
        mConn.ConnectionString = mConnectionString
    End If
    
    On Error GoTo errHandler
        If mConn.State = adStateClosed Then
            mConn.Open
        End If
    
        connectionSuccessful = (mConn.State = adStateOpen)
    On Error GoTo 0
    
    Exit Function
errHandler:
    Debug.Print "Error: Connection unsuccessful"
    connectionSuccessful = False
End Function

Private Sub mASyncConn_ExecuteComplete(ByVal RecordsAffected As Long, ByVal pError As ADODB.Error, adStatus As ADODB.EventStatusEnum, ByVal pCommand As ADODB.Command, ByVal pRecordset As ADODB.Recordset, ByVal pConnection As ADODB.Connection)
    If mProcedureAfterQuery <> "" Then
        Call Application.Run(mProcedureAfterQuery, pRecordset)
    End If
End Sub

Answer 1

Private Function connectionSuccessful() As Boolean

The name suggest that you are testing to see if the Connection has already been opened when in fact it is used to opening the Connection and test if it was successful.

Private Function OpenConnection() As Boolean   

This name tells you that you are opening a Connection. Since the return type is Boolean, it is natural to assume that the function will return True only if the Connection was successful.

Having the error handler escape errors and print a message to the Immediate Window is counter productive. As a developer I don't instinctively look to the Immediate Window for error messages. As a user I will notify the developer of the error message that was raised down the line and not at the point of impact. Considering that your code uses callback procedures, there is no guarantee that an error will ever be raised. The only thing that is certain is that there are going to be problems somewhere down the line.

You should definitely raise a custom error it the mConnectionString is not set. A custom error message for the failed connection is not necessary (if you remove the error handler) because an ADODB error will be thrown at the point where this procedure was called.

Public Sub AsyncExecute()

Consider raising an error if the callback procedure is not set.

Private Sub Class_Terminate()

This method should be used to close the connection.

mConn, mASyncConn, and mSyncConn

There is no need to use three different Connection variable. You are doing more work and obfuscating the code. Using a variable such as AsyncMode As Boolean will give you the same feedback and simplify the code making it easier to read.

Naming Conventions

Having value and execute lower case changes the case for all other variables and properties with the same names. For this reason, I use Pascal Case for all my variables that do not have some sort of prefix.

Mathieu Guindon's Factories: Parameterized Object Initialization

Other possible improvements

A public event would allow you to use cQueryable in other custom classes.

Public Event AsyncExecuteComplete(pRecordset As Recordset)

The ability to chain queries together seems like a natural fit.

Public Function NextQuery(Queryable AS cQueryable) AS cQueryable
   Set NextQuery = Queryable 
   Set mQueryable = Queryable 
End Function

This will allow you to run multiple queries in order without the need of multiple callback.

CreateTempQuery.NextQuery(FillTempTableQuery).NextQuery(UpdateEmployeesTableQuery)