SqlCommand closing connections properly

Question

I have been working on an old project recently. I found in the project that previous colleagues have created some common methods to use for to open connection to database. But I have a doubt about the process whether it really handles connection by IDispose once the query completes.

Here is the methods:

Connect database.

/// <summary>
///     This method creates a connection object to the database.
/// </summary>
/// <returns>a new SQL Connection</returns>
public SqlConnection ConnectDB()
{
    var db = new SqlConnection(ConnectionString);
    db.Open();
    return db;
} 

public SqlDataReader GetDataReader(SqlCommand query)
{
    var db = ConnectDB();
    query.Connection = db;
    var reader = query.ExecuteReader();
    return reader;
}

Then we use GetDataReader as below

var queryProduct= new SqlCommand
{
    CommandText = "SELECT DISTINCT ProductName FROM Products"
};   

using (var reader = Interaction.GetDataReader(queryProduct))
{
    while (reader.Read())
    {
         var b = new Product
         {
              ProductName = reader.GetString(reader.GetOrdinal("ProductName"))
         };
         products.Add(b);
    } 
}

Do you think this process would release all the connection correctly?

Answer 1

The code isn't safe. Disposing/closing the reader doesn't automatically close the connection, as you may want to execute multiple commands on the same connection. Even if you use the override that does close the connection, exceptions that may occur before you enter the using block will leave the connection open.

The typical way is to wrap the connection, command and reader in using statements:

using(var con=new SqlConnection(connectionString))
using(var command=new SqlCommand(sql,con))
{
    con.Open();
    using(var reader=command.ExecuteReader())
    {
    ....
    }
}

Answer 2

SqlConnection also implements IDisposable interface, so you have to close the connection too. So you should also wrap the connection in using block.

Answer 3

No, your code doesn't release all the connections correctly as they have confirmed in other answers. Whereas modify an old program can be a nightmare I suggest an approach like this

class Program
{
    static IEnumerable<SqlDataReader> InteractionGetData(string query)
    {
        using (var connection = new SqlConnection(@"Data Source=localhost;Integrated Security=SSPI;"))
        {
            connection.Open();

            using (var command = new SqlCommand(query, connection))
            {
                using (var reader = command.ExecuteReader())
                {
                    while (reader.Read())
                    {
                        yield return reader;
                    }
                }
            }
        }
    }

    static void Main(string[] args)
    {
        foreach (var reader in InteractionGetData("SELECT DISTINCT ProductName FROM Products"))
        {
            Console.WriteLine(reader.GetString(reader.GetOrdinal("ProductName")));
        }
    }
}