Which pattern is better for SqlConnection object?

Question

Hi! Which pattern is better for SqlConnection object? Which is better in performance? Do you offer any other pattern?

class DataAccess1 : IDisposable
{
    private SqlConnection connection;

    public DataAccess1(string connectionString)
    {
        connection = new SqlConnection(connectionString);
    }

    public void Execute(string query)
    {
        using (SqlCommand command = connection.CreateCommand())
        {
            command.CommandText = query;
            command.CommandType = CommandType.Text;
            // ...

            command.Connection.Open();
            command.ExecuteNonQuery();
            command.Connection.Close();
        }
    }

    public void Dispose()
    {
        connection.Dispose();
    }
}

VS

class DataAccess2 : IDisposable
{
    private string connectionString;

    public DataAccess2(string connectionString)
    {
        this.connectionString = connectionString;
    }

    public void Execute(string query)
    {
        using (SqlConnection connection = new SqlConnection(connectionString))
        {
            SqlCommand command = connection.CreateCommand();
            command.CommandText = query;
            command.CommandType = CommandType.Text;
            // ...

            command.Connection.Open();
            command.ExecuteNonQuery();
            command.Connection.Close();
        }
    }

    public void Dispose()
    {            
    }
}

Answer 1

There's no real way to answer this question. The short, canonical answer is that the connection should stay alive for the lifetime of your unit of work. Because we have no way of knowing how DataAccess is used (does it exist for the lifetime of your application, or do you instantiate it and dispose it whenever you do something?), it's impossible to give a concrete answer.

That being said, I would recommend the first pattern, but instantiate and dispose of your DataAccess object as needed; don't keep it around longer than necessary.

Answer 2

I think it depends on how your DataAccess object is intended to be used, if it's used within a 'using' clause then the connection is guaranteed to be disposed of after it's done.

But in general I prefer the second pattern as the sql connection is created and disposed of within the Execute method so it's less likely to be left open when you forget to dispose of your DataAccess object.

Considering that sql connection can be a scarse resource I think every attempt should be made to ensure that they're not wasted.

Answer 3

Suggest going with DataAccess2. It's a personal preference though. Some might even suggest your class be static. It'd be difficult to say that one is more performant than the other. You're on the path of IDisposable, which is great.

I'd be happy to read and maintain both styles shown above in your question.

Consider having your DAL be able to read the connection string from a .config as well, rather than exclusively allowing the value to be passed in the constructor.

public DataAccess2(string connStr)
{
    this.connectionString = connStr;
}
public DataAccess2()
{
    this.connectionString = 
            ConfigurationManager.ConnectionStrings["foo"].ConnectionString;
}

Consider wrapping your SqlCommand in a using as well.

using (var conn = new SqlConnection(connectionString))
{
    using(var cmd = conn.CreateCommand())
    {

    }
}

Answer 4

The first will result in errors if you make concurrent calls. The second will ensure you use a clean connection for each command resulting in more connections being made.

I agree with the statements above that it depends on the scenario for use, to get over the problem related to the first I have a wrapper that needs to use such a pattern so I set a field value boolean to show that a command is being executed on the connection already then "queue" the next command for execution.

There will of course be situations where you may prefer to use multiple connections ...