Handling of open database connection and calling shared function from WebAPI

Question

I'm working on an ASP.NET project using VB.NET that uses Dapper and the code as implemented so far runs fine with just me testing. In the example below, Dapper calls a stored proc.

But I am wondering primarily right now if the method of generating an open connection (see dbConnFactory below) implemented in the Domain project is not good practice.

Also, can there be pitfalls to calling a Shared function from .NET's WebAPI, whether it the function is for GET, PUT, POST, or Delete?

AutomoblieDomain project

Imports System.Data.Common
Imports Dapper 
Namespace DAL
 Public Class DomainSettings
 Public Shared Property CustomConnectionString As String 
 End Class
 Public Class dbConnFactory
 Public Shared Function GetOpenConnection() As DbConnection
 Dim connection = New SqlClient.SqlConnection(CustomConnectionString)
 connection.Open()
 Return connection
 End Function
 End Class
 Public Class CarTypes
 Public Property CarTypeID As Integer
 Public Property CarTypeText As String
 Public Shared Function GetList() As IEnumerable(Of CarTypes)
 Using conn = dbConnFactory.GetOpenConnection()
 Dim _list = conn.Query(Of CarTypes)("dbo.CarTypes_GetList", CommandType.StoredProcedure).ToList() 
 Return _list
 End Using
 End Function
 End Class
End Namespace

Web UI Project

Imports System.Net
Imports System.Web.Http
Imports AutomobileDomain
Public Class CarTypesController
 Inherits ApiController
 <HttpGet()>
 Public Function GetList() As IEnumerable(Of DAL.CarTypes)
 Return DAL.CarTypes.GetList()
 End Function
End Class

• dreza: Just an opinion but if your GetList() method is returning a list then perhaps you should specify an IList instead of an IEnumerable. That way it's explict to the caller that there is no delayed execution occuring once they have the data back

Answer 1

According to MSDN:

"We recommend that you always close the connection when you are finished using it in order for the connection to be returned to the pool. You can do this using either the Close or Dispose methods of the Connection object, or by opening all connections inside of a using statement in C#, or a Using statement in Visual Basic." http://msdn.microsoft.com/en-us/library/8xx3tyca(v=VS.80).aspx - Section: "Adding Connections"

The way your code is written, (connection being delivered via a factory method) does look a little risky. However, the way MSDN describes it, at the end of the Using block, the connection object will be automatically closed and disposed.

• mg1075: Thanks; I saw the msdn documentation, and was looking for confirmation as to whether or not it was completely foolish to use the factory method. It does seem like over optimization and I'll stay away from that implementation, but now I'm wondering about calling the Shared function from the web api controller...
• TGolisch: Using a factory method to return an open connection is a risky endeavor because you have to trust that anyone using it will always remember to close/dispose it. It is a violation of atomicity (an object is not self-dependent any more). Make sure you are vigilant with your code-reviews! One approach that might help would be to move GetOpenConnection() to a private method in a base class and have your CarTypes class (and others) inherit from it. Then you would limit the use of that factory method. Otherwise, you have to search all of your code for calls to GetOpenConnection()
• Roman: @mg1075 I agree with TGolisch completely. Do not expose a method that returns an Open connection. I don't see anything wrong with just returning a connection object.