21

Was wondering if it is recomended to pass a database connection object around(to other modules) or let the method (in the other module) take care of setting it up. I am leaning toward letting the method set it up as to not have to check the state of the connection before using it, and just having the caller pass any needed data to the calling method that would be needed to setup the connection.

DOK
  • 32,337
  • 7
  • 60
  • 92
CSharpAtl
  • 7,374
  • 8
  • 39
  • 53

9 Answers9

13

Personally I like to use tightly scoped connections; open them late, use them, and close them (in a "using" block, all within the local method). Connection pooling will deal with re-using the connection in most cases, so there is no real overhead in this approach.

The main advantage in passing connections used to be so that you could pass the transaction around; however, TransactionScope is a simpler way of sharing a transaction between methods.

Since the classes are implementation specific, I'd write each to open it's own native transaction. Otherwise, you can use the ado.net factory methods to create the appropriate type from the config file (the provider name).

Community
  • 1
  • 1
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • 4
    If you use TransactionScope to share a transaction between methods that use different connections, the transaction becomes a distributed transaction with the associated overhead. So it's still often better to pass a connection (and its transaction) between methods, while keeping the scope tight. – Joe Oct 30 '08 at 21:20
  • Thats another reason I like using TLS for this stuff – Sam Saffron Oct 30 '08 at 21:34
  • This is the way we usually do it, and the way I prefer it. – CSharpAtl Oct 31 '08 at 11:29
  • Well, if you want to use a single Transaction between two connections, then it HAS to be a distributed transaction. So that isn't really a downside. – Jared Moore Apr 27 '11 at 21:44
  • @MarcGravell even if you have many objects? e.g. IShop that pass List objects... each IItem should have the Connection String and Open it's own connection? – Tomer W Dec 09 '16 at 10:27
  • @TomerW I didn't say that at all, and that would be a terrible idea. Why would an item **ever** need to hold a connection? that is not what an item (usually) is meant to encapsulate... In fact, I said the exact opposite: **don't** hold connections. Open one **when you need it** and then release it. That rarely involves fields – Marc Gravell Dec 09 '16 at 10:45
  • @MarcGravell i think i didn't explain myself... Item has it's values cached in Database, but can perform some operations on it's db record. so in an operation of updating the entire store, what would happen is for each item: Open connection, alter DB, Close connection. – Tomer W Dec 09 '16 at 12:10
10

Personally, I like storing a stack of my current open connection and transactions on top of the Thread Local Storage using SetData and GetData. I define a class that manages my connections to the database and allow it to use the dispose pattern. This saves me the need to pass connections and transactions around, which is something that I think clutters and complicates the code.

I would strongly recommend against leaving it up to the methods to open connections every time they need data. It will leads to a really bad situation where it is both hard to manage transactions throughout the application and too many connections are opened and closed (I know about connection pooling, it is still more expensive to look up a connection from the pool than it is to reuse an object)

So I end up having something along these lines (totally untested):

class DatabaseContext : IDisposable {

    List<DatabaseContext> currentContexts;
    SqlConnection connection;
    bool first = false; 

    DatabaseContext (List<DatabaseContext> contexts)
    {
        currentContexts = contexts;
        if (contexts.Count == 0)
        {
            connection = new SqlConnection(); // fill in info 
            connection.Open();
            first = true;
        }
        else
        {
            connection = contexts.First().connection;
        }

        contexts.Add(this);
    }

   static List<DatabaseContext> DatabaseContexts {
        get
        {
            var contexts = CallContext.GetData("contexts") as List<DatabaseContext>;
            if (contexts == null)
            {
                contexts = new List<DatabaseContext>();
                CallContext.SetData("contexts", contexts);
            }
            return contexts;
        }
    }

    public static DatabaseContext GetOpenConnection() 
    {
        return new DatabaseContext(DatabaseContexts);
    }


    public SqlCommand CreateCommand(string sql)
    {
        var cmd = new SqlCommand(sql);
        cmd.Connection = connection;
        return cmd;
    }

    public void Dispose()
    {
        if (first)
        {
            connection.Close();
        }
        currentContexts.Remove(this);
    }
}



void Test()
{
    // connection is opened here
    using (var ctx = DatabaseContext.GetOpenConnection())
    {
        using (var cmd = ctx.CreateCommand("select 1"))
        {
            cmd.ExecuteNonQuery(); 
        }

        Test2(); 
    }
    // closed after dispose
}

void Test2()
{
    // reuse existing connection 
    using (var ctx = DatabaseContext.GetOpenConnection())
    {
        using (var cmd = ctx.CreateCommand("select 2"))
        {
            cmd.ExecuteNonQuery();
        }
    }
    // leaves connection open
}
Sam Saffron
  • 128,308
  • 78
  • 326
  • 506
  • Out of curiosity, what prevents the Dispose() call in Test2 from closing the connection opened in Test? I'm guessing DatabaseContext would have to keep track of the number of clients that asked for the connection and only allow Dispose to close the connection when only 1 request is open. Thus it would follow that Dispose would also reduce the number of clients inside DatabaseContext. Seems like an opportunity for undue coupling (could be fixed probably). Also, what happens when you need to share a tran across threads? – xero Jan 20 '10 at 17:18
  • @xero, tran across threads is a fairly rare condition, so you handle it manually. It is a fairly dangerous game to play. When you create a context it knows if its the connection owner or not and then dispose acts appropriately – Sam Saffron Jan 20 '10 at 21:01
8

For automated testing purposes, it's usually easier to pass it in. This is called dependency injection.

When you need to write tests, you can create a mock database connection object and pass that instead of the real one. That way, your automated tests won't rely on an actual database that needs to be repopulated with data every time.

Micah
  • 17,584
  • 8
  • 40
  • 46
1

I personally work to centralize my data access as much as possible, however, if not possible I ALWAYS open a new connection in the other classes, as I find that there are too many other things that can get in the way when passing the actual connection object.

Mitchel Sellers
  • 62,228
  • 14
  • 110
  • 173
1

Setting up the connection is potentially expensive and potentially adds a round trip. So, again, potentially, the better design is to pass the connection object.

I say potentially, because if you are a Microsoft ADO app, you are probably using a connection pool....

Corey Trager
  • 22,649
  • 18
  • 83
  • 121
1

Here is a little more insight into this problem. I have a class that manages db connections, and have 2 classes that implement an interface. One of the classes is for SQL and the other is of OLAP. The manager is the one that knows which connection to use, so it could pass the exact connection to the type, or the type can create his own connection.

CSharpAtl
  • 7,374
  • 8
  • 39
  • 53
1

You can pass connection objects without any problem (for instance Microsoft Enterprise Library allows static method calls passing in a connection) or you could manage it externally its up to your design, there are not direct technical tradeoffs.

Be careful for portability not to pass an specific connection if your solution will be ported to other databases (meaning don´t pass a SqlConnection it you plan to work with other databases)

Simara
  • 1,135
  • 1
  • 12
  • 14
1

I would suggest that you distinguish between the connection object and its state (open, closed).

You can have a single method (or property) that reads the connection string from web.config. Using the same version of the connection string every time ensures that you will benefit from connection pooling.

Call that method when you need to open a connection. At the very last moment, after setting up all of the SqlCommand properties, open the connection, use it, and then close it. In C#, you can use the using statement to make sure the connection is closed. If not, be sure to close the connection in a finally block.

DOK
  • 32,337
  • 7
  • 60
  • 92
0

I would use the web.config

<configuration>
    <connectionStrings>
        <add name="conn1" providerName="System.Data.SqlClient" connectionString="string here" />
        <add name="conn2" providerName="System.Data.SqlClient" connectionString="string here" />
    </connectionStrings>
</configuration>

Then you can reference it from anywhere in the application

craigmoliver
  • 6,499
  • 12
  • 49
  • 90