1
\$\begingroup\$

I'm using dapper-dot-net for data access in my ASP.NET MVC application.
So in order to supply a connection string I've came up with the following pattern:

public static class ConnectionStringManager
{
    private static readonly SqlConnection conn;

    static ConnectionStringManager()
    {
        //initialize the connection with value from web.config file
        conn = new SqlConnection(ConfigurationManager.ConnectionStrings["MyConnName"].ConnectionString);
    }

    public static SqlConnection GetOpenConnection()
    {
        if (conn.State != System.Data.ConnectionState.Open)
            conn.Open();

        return conn;
    }
}

And a sample use of it with in a query:

public Customer GetCustomerById(int id)
{
    string sql = @"select * from Customer where Id = @id";
    using (var conn = ConnectionStringManager.GetOpenConnection())
    {
        Customer cust = conn.Query<Customer>(sql, new { id }).FirstOrDefault();
        return cust;
    }
}

So my question is if this is a good approach or not, and if not, what would you suggest to do different?

\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

You create the SqlConnection only once, in the type constructor of ConnectionStringManager. Then you wrap its usage in a using block which is good practice but will dispose the object when it leaves the scope, so any subsequent usage will result in an ObjectDisposedException.

Instead, create a new connection in the GetOpenConnection method (maybe rename it to CreateConnection or similar in order to reflect the fact that there's work being done).

\$\endgroup\$
6
  • \$\begingroup\$ Care to elaborate the downvote? \$\endgroup\$
    – nodots
    Commented Mar 31, 2016 at 14:35
  • \$\begingroup\$ Whoa, that's a long shot and I highly doubt it. In that case (single operation), why even bother with a static manager class at all? \$\endgroup\$
    – nodots
    Commented Mar 31, 2016 at 14:44
  • \$\begingroup\$ Yeah and in that future scenario my point still stands! With the presented usage pattern the GetOpenConnection method cannot guarantee to do what you'd think it does, judging by its name. It will give you an open connection once and only once. \$\endgroup\$
    – nodots
    Commented Mar 31, 2016 at 14:51
  • \$\begingroup\$ Ok, last attempt. Do you really not understand what I'm saying? GetOpenConnection claims to give you an open connection on each call. With the presented usage pattern (using block) -- which is good practice -- you get an open connection on the first call only. One. Single. Time. On each subsequent call, the method throws an exception. \$\endgroup\$
    – nodots
    Commented Mar 31, 2016 at 15:42
  • \$\begingroup\$ because it's a static object.... \$\endgroup\$
    – Malachi
    Commented Mar 31, 2016 at 15:43
1
\$\begingroup\$

I don't know about calling this a ConnectionStringManager as you are not really managing the string for the connection, you are managing the connection itself, well rather opening a connection if the connection is not open.

I don't know if I would use a static class to do this.

You could probably use a factory pattern for this, but you would have to accept a parameter of the string used for the configuration location that holds the connection. But, what happens if you don't hold the connection strings in the same location?

Here is what it would look like if you created this as a Factory

public static class ConnectionFactory
{
    public static SqlConnection GetOpenConnection(string connectionName)
    {
        var conn = new SqlConnection(ConfigurationManager.ConnectionStrings[connectionName].ConnectionString);
        conn.Open();
        return conn;
    }
}

Your usage is the same, except for the name changes and passing in the connectionName this decouples the method from the caller, you could change the connection you want to use for this particular method and not affect any other methods that use this static method.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.