Connection Class terminates prior to going out of scope

[jbStovers]

I'm trying to use a class (called SeasFcstDbConn) with an ADO connection as its property to quickly connect to my SQL database.


One use of this class is shown below, where a method of another class instantiates the connection class and uses it to open a recordset.
The problem is that when the recordset.open method runs, SeasFcstDbConn not only fires the initialize event, but the terminate event as well, which closes the connection. This causes the next line (.AddNew) in the calling procedure to fail.

Here is the calling procedure, which exists in a different class module.

 Public Sub InsertDb()
     Dim cn As New SeasFcstDbConn
     Dim rst As ADODB.Recordset
     
     Set rst = New ADODB.Recordset
     With rst
         .Open "WAVE_DETAIL", cn, adOpenDynamic, adLockOptimistic
         .AddNew         'THIS LINE FAILS
         !Seas = pSeas
         !Item = pItem
         !Cust = pCust
         !Amg = pAmgQty
         !Amg_Conf = pAmgConf
         !Fcst = pFcst
         !Beg_Fcst_Dt = pBegFcst
         !End_Fcst_Dt = pEndFcst
         .Update
         .Close
     End With
     Set rst = Nothing
     Set cn = Nothing
 End Sub

Here is the SeasFcstDbConn Class:

 Option Compare Database
 Private cn As ADODB.Connection

 Property Get Conn() As ADODB.Connection
     Set Conn = cn
 End Property

 Private Sub Class_Initialize()      'Fires as it should on the rst.Open method
     Set cn = New ADODB.Connection
     Const strConnect As String = _
         "DRIVER=SQL Server;" & _
         "SERVER=KCM1WN4VR1;" & _
         "DATABASE=SeasFcstDEV;" & _
         "Trusted_Connection=Yes"
     cn.Open strConnect
 End Sub

 Private Sub Class_Terminate()       'ALSO FIRES ON THE rst.Open METHOD (WHY???)
     If cn.State = adStateOpen Then cn.Close
     Set cn = Nothing
 End Sub

The Terminate event does NOT fire if I write the rst.Open command as:

     With rst
         .Open "WAVE_DETAIL", cn.Conn, adOpenDynamic, adLockOptimistic

This works fine, but I don't want to have to retrieve the Conn property since the class itself is supposed to represent a connection. I specified "Property Get Conn()" as the default procedure for the SeasFcstDbConn class by opening the module in Notepad and adding the line "Attribute Item.VB_UserMemId=0" to that proc. And I confirmed that this worked by checking the object browser (the Conn property has a little blue dot which shows that it is the default). Still the problem is not resolved.

Maybe I am missing something really simple here, but I expected the Terminate event to fire at the "Set cn = Nothing" command of the calling proc, instead of during the rst.Open command.
I'd greatly appreciate any advice on this one.

[ItsMe]

Why not include an actual connection string instead of an object in your Open method?
.Open "WAVE_DETAIL", strConnect, adOpenDynamic, adLockOptimistic

Include this .Open method in class SeasFcstDbConn. You can add additional constructors or a function to manage arguments like adLockOptimistic and your magic string WAVE_DETAIL. All of this can be managed by SeasFcstDbConn. Then, the Terminate Procedure should not execute until .Close of the SeasFcstDbConn class.

Dim cn As New SeasFcstDbConn
...
cn.Close   'Execute termination procedure
set cn = nothing

[jbStovers]

Thank you for the reply. I'm a little confused. An Open method in the SeasFcstDbConn class would look like cn.Open. Are you suggesting I create a method in the SeasFcstDbConn class to return a recordset? And maybe naming it .OpenRecordset? That seems to make good sense.

This is my first time implementing classes in an app, and I've been struggling with how to separate the data access functions from the app logic. Since I want to eventually use my design to convert the app to a web solution, I was initially hoping to keep all the connections and recordsets inside the SeasFcstDbConn class, and just return strings or arrays to the calling procedures. Obviously, my example above includes a recordset object outside the SeasFcstDbConn class. However, I actually have a few SeasFcstDbConn methods that query the database and return delimited strings to populate forms. With your suggestion, it looks like maybe I should forget that approach and have SeasFcstDbConn return recordsets which the calling procedure can convert to string if need be. What do you think?

BTW, the magic string "WAVE_DETAIL" is one of my tables .

[ItsMe]

...Are you suggesting I create a method in the SeasFcstDbConn class to return a recordset? And maybe naming it .OpenRecordset? That seems to make good sense...

Yes, in a nutshell. All your class seems to do right now is provide a string, a connection string. So I would build it out to provide recordset objects, if that is what you are after. This way, your Class can participate as a persistence layer within your application.

I am a little busy right now, but I can provide you some examples later this afternoon.

[jbStovers]

I would build it out to provide recordset objects... This way, your Class can participate as a persistence layer within your application

This is the part I don't seem to be able to understand. The calling proc initializes/opens the connection object and assigns it to the variable cn. I would expect the cn object to persist until I set it = nothing.

Thanks a lot for your help.

[jbStovers]

I know this is getting more into the topic of design, but would you recommend using just a couple of methods in the connection class to build queries based on arguments, OR would you build a different method for every specific recordset needed. In other words, should I write the SQL in various other procs and pass it as an argument to the connection object, or put all the SQL in the connection class? I lean toward the latter... seems like it would simplify the code in all the other procs.

[ItsMe]

I am not perfectly clear what you want to do with your class. It seems to me that you want a class or a class library to abstract a persistence layer. In other words, you could apply this class or class library to more than one application. The class would represent a connection to the data source. I am not an excellent application architect but I have studied it a little. I think I recognize you aiming for a persistence layer.

The reason why your class is closing in an unexpected way is that you are not interacting with it the way you think you are, the way you are trying to. Maybe these examples will shed some light. Maybe they will muddy the waters. Anyway ...

I would start with some enums so you can communicate from the application layer to the other layers. For instance, your table names could be managed with enums, providing strong types. You could place something like the following in a Standard module. The result is you can get a strongly typed string using EnumTableName(Table1)

Enum EnumTable

    Table1 = 1
    Table2 = 2
    Table3 = 3
    Table4 = 4

End Enum

Public Function EnumTableName(TableName As EnumTable) As String

    Select Case TableName
    
        Case 1
            EnumTableName = "Table1"
        Case 2
            EnumTableName = "Table2"
        Case 3
            EnumTableName = "Table3"
        Case 4
            EnumTableName = "Table4"
    
    End Select
    
End Function

I am not an expert with ADO and I don't have an opportunity to test the following code. The idea is to give you some food for thought and help you with your abstractions. You want to make a determination how to avoid leaky abstractions. How and where are you going to manage your recordsets. I feel your question is regarding why your ADO connection is closing. Focusing on a proper abstraction and properly interacting with your class may bring you closer to a solution.

I would use something like this in a Class to start with.

Option Compare Database

Private Const p_ConnectionString As String = _
        "DRIVER=SQL Server;" & _
        "SERVER=KCM1WN4VR1;" & _
        "DATABASE=SeasFcstDEV;" & _
        "Trusted_Connection=Yes"

 Private p_Conn As ADODB.Connection
 Private p_DataSetType As Integer
 Private p_LockType As Integer
 
 
Public Property Let DataSetType(ByVal DataSetType As Integer)

    If IsNull(DataSetType) Then
        p_DataSetType = 0 'Some Default Value
    Else
        p_DataSetType = DataSetType
    End If
    
End Property


Public Property Let LockType(ByVal LockType As Integer)

    If IsNull(LockType) Then
        p_LockType = 0 'Some Default Value
    Else
        p_LockType = LockType
    End If
    
End Property

Public Function GetRecordSet(ByVal DomainName As EnumTable) As ADODB.Recordset
    Set GetRecordSet = New ADODB.Recordset
    GetRecordSet.Open EnumTableName(DomainName), p_Conn, DataSetType, LockType
End Function
 
 
Private Sub Class_Initialize()
    Set p_Conn = New ADODB.Connection
    p_Conn.Open p_ConnectionString
End Sub
 
 
 Private Sub Class_Terminate()
     If p_Conn.State = adStateOpen Then p_Conn.Close
     Set p_Conn = Nothing
 End Sub

Then, behind a form and in your application layer you might do something like ...

Dim rs As New SeasFcstDbConn
Dim MyRs As New ADODB.Recordset

With rs

.DataSetType = adOpenDynamic
.LockType = adLockOptimistic
'Rather than pass it as a local RS object you might want to
'include additional constructors in your original class for various columns
Set MyRs = .GetRecordSet(Table1)

Wend

It is just food for thought. Your class is not behaving the way you expect because you are not talking to it the way you believe you are. Hopefully you can reverse engineer the above examples and doing so will help provide some insight.