Calling Adapter.CloseConnection(), and so on

Posts   
 
    
mkamoski avatar
mkamoski
User
Posts: 116
Joined: 06-Dec-2005
# Posted on: 11-Apr-2006 18:18:53   

All--

Please help.

This is a question about calling Adapter.CloseConnection().

In general, the questions are...

(1). When should one call Adapter.CloseConnection()?

(2). When shouldn't one call Adapter.CloseConnection()?

(3). Should one call Adapter.CloseConnection(), just in case? Does it hurt? Does it help?

(4). Should one set the Adapter to Nothing in a Finally block, just in case? Does it hurt? Does it help?

(5). I see the developers closing the connection on our team. (See the code directly below.) Is it OK or NOT-OK? Why or why not? Is there a better way?


Public Overloads Sub SomeMethod()
    Dim myAdapter As DatabaseSpecific.DataAccessAdapter = Nothing

    Try
        '...some code...

        myAdapter = New DatabaseSpecific.DataAccessAdapter

        '...use the adapter here...
    Catch ex As Exception
        Throw ex
    Finally
        If (Not myAdapter Is Nothing) Then
            myAdapter.CloseConnection()
            myAdapter = Nothing
        End If
    End Try
End Sub

(6). In the Adapter sample for VB, it looks like Dispose() is used instead. (See the code directly below.) How does this differ from closing the connection and setting it to Nothing?


Private Function RemoveOrder(ByVal orderToRemove As OrderEntity) As Boolean
    Dim adapter As New DataAccessAdapter
    Dim toReturn As Boolean = False

    adapter.StartTransaction(System.Data.IsolationLevel.ReadCommitted, "RemoveOrder")
    
    Try
        ' first remove the orderdetail rows of this order
        adapter.DeleteEntityCollection(orderToRemove.OrderDetails)

        ' remove the order itself
        toReturn = adapter.DeleteEntity(orderToRemove)

        ' done
        adapter.Commit()
    Catch
    
    Finally
        adapter.Dispose()
    End Try

    Return toReturn
End Function        

What do you think?

Please advise.

Thank you.

--Mark Kamoski

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39928
Joined: 17-Aug-2003
# Posted on: 11-Apr-2006 19:21:02   

mkamoski wrote:

All--

Please help.

This is a question about calling Adapter.CloseConnection().

In general, the questions are...

(1). When should one call Adapter.CloseConnection()?

Only if you manually called OpenConnection() AND/OR you specified KeepConnectionOpen as true.

(2). When shouldn't one call Adapter.CloseConnection()?

When you have a transaction running and you need the open connection wink . Seriously: it doesn't matter. if the connection is open, it's closed. If there's no connection, nothing happens.

(3). Should one call Adapter.CloseConnection(), just in case? Does it hurt? Does it help?

It doesn't hurt, but you don't need to. Let adapter figure out when to open a connection.

ONLY if you specified to keep open the connection, either in the constructor of dataaccessadapter or through the property, you should call CloseConnection(), which is logical as adapter doesn't know when to close it as you specified it should be kept open!

(4). Should one set the Adapter to Nothing in a Finally block, just in case? Does it hurt? Does it help?

One should call Dispose in a final block. Thus preferred, do: using(DataAccessAdapter adapter = new DataAccessAdapter()) { //... code }

which calls dispose at the end, cleaning everything up (including open transactions/connections etc.)

(5). I see the developers closing the connection on our team. (See the code directly below.) Is it OK or NOT-OK? Why or why not? Is there a better way?


Public Overloads Sub SomeMethod()
    Dim myAdapter As DatabaseSpecific.DataAccessAdapter = Nothing

    Try
        '...some code...

        myAdapter = New DatabaseSpecific.DataAccessAdapter

        '...use the adapter here...
    Catch ex As Exception
        Throw ex
    Finally
        If (Not myAdapter Is Nothing) Then
            myAdapter.CloseConnection()
            myAdapter = Nothing
        End If
    End Try
End Sub

typical VB6 -> .NET code. No offence meant, it just has old-VB habbits included. wink The Catch Ex / throw ex is bad though. It destructs the stacktrace!

You should do this instead.

    
Public Overloads Sub SomeMethod()
    Dim myAdapter As DatabaseSpecific.DataAccessAdapter = Nothing

    Try
        '...some code...

        myAdapter = New DatabaseSpecific.DataAccessAdapter

        '...use the adapter here...
    Finally
        If (Not myAdapter Is Nothing) Then
            myAdapter.Dispose()
        End If
    End Try
End Sub

(6). In the Adapter sample for VB, it looks like Dispose() is used instead. (See the code directly below.) How does this differ from closing the connection and setting it to Nothing?


Private Function RemoveOrder(ByVal orderToRemove As OrderEntity) As Boolean
    Dim adapter As New DataAccessAdapter
    Dim toReturn As Boolean = False

    adapter.StartTransaction(System.Data.IsolationLevel.ReadCommitted, "RemoveOrder")
    
    Try
        ' first remove the orderdetail rows of this order
        adapter.DeleteEntityCollection(orderToRemove.OrderDetails)

        ' remove the order itself
        toReturn = adapter.DeleteEntity(orderToRemove)

        ' done
        adapter.Commit()
    Catch
    
    Finally
        adapter.Dispose()
    End Try

    Return toReturn
End Function        

Not the correct code, you don't rollback on an exception. This eventually happens in the finally block as dispose calls Reset which rollsback a dangling transaction, but that's not what should be done.

Better is:


Private Function RemoveOrder(ByVal orderToRemove As OrderEntity) As Boolean
    Dim adapter As New DataAccessAdapter
    Dim toReturn As Boolean = False

    adapter.StartTransaction(System.Data.IsolationLevel.ReadCommitted, "RemoveOrder")
    Try
        ' first remove the orderdetail rows of this order
        adapter.DeleteEntityCollection(orderToRemove.OrderDetails)

        ' remove the order itself
        toReturn = adapter.DeleteEntity(orderToRemove)

        ' done
        adapter.Commit()
    Catch
    adapter.RollBack()
    Throw
    Finally
        adapter.Dispose()
    End Try
    Return toReturn
End Function 

Frans Bouma | Lead developer LLBLGen Pro
mkamoski avatar
mkamoski
User
Posts: 116
Joined: 06-Dec-2005
# Posted on: 12-Apr-2006 17:12:25   

Otis--

Great.

Dispose it is.

I really appreciate the details and clarity.

BTW, regarding your comment above that says "Not the correct code, you don't rollback on an exception", that is fine. I understand I do see the issue. I am glad that you pointed that out because I had missed it. However, just as an FYI item, I should note that that code was copied from the "NorthwindExampleVB" sample project downloaded from the http://www.LLBLGen.com site. It is found in the CustomerManager.RemoveOrder() method.

That said, I should also note that downloaded the NorthwindExampleVB about 2 months ago. Maybe I have an outdated version of it. Regardless, I wanted to let you know, just in case.

Incidentally, that NorthwindExampleVB sample is great and our team finds it VERY helpful. We use it a lot.

Thanks again.

--Mark Kamoski

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39928
Joined: 17-Aug-2003
# Posted on: 12-Apr-2006 17:35:10   

touche! flushed simple_smile

I translated the VB.NET code with an automated translated from the C# code, and indeed it is perhaps wrong. It's not the newest code on the block (most of it is written in 2003 if I'm not mistaken).

I'll look into updating that code. Thanks for the heads up simple_smile

Frans Bouma | Lead developer LLBLGen Pro