Question

I have a code like this in my program and I believe that it's not closing the connection after that the data is getting filled in.

public static string ConnectionInfo = System.Configuration.ConfigurationManager.ConnectionStrings["Default"].ConnectionString;
public static DataTable ExecuteQuery(string query, string table)
    {
        SqlConnection cnn = new SqlConnection(ConnectionInfo);
        SqlDataAdapter Adp = new SqlDataAdapter(query, cnn);
        DataSet Ds = new DataSet();
        Adp.Fill(Ds, table);
        return Ds.Tables[table];
    }

Is there any problem in this code ?

Was it helpful?

Solution

The only problem is that you are not using the using statement for the SqlConnection and the DataAdapter. However, DbDataAdapter.Fill opens and closes the connection implicitely.

public static DataTable ExecuteQuery(string query, string table)
{
    using(SqlConnection cnn = new SqlConnection(ConnectionInfo))
    using(SqlDataAdapter Adp = new SqlDataAdapter(query, cnn))
    {
        DataTable tbl = new DataTable();
        Adp.Fill(tbl);
        return tbl;
    }
}

The connection object associated with the SELECT statement must be valid, but it does not need to be open. If the connection is closed before Fill is called, it is opened to retrieve data, then closed. If the connection is open before Fill is called, it remains open.

Note that

  • the using statement will close the connection implicitely even on error
  • i have used DataAdapter.Fill(DataTable) because you're using a single table anyway

Edit: i've only just noticed that you are using a parameter for the table-name. You can also use DbDataAdapter.Fill(DataSet, String) instead. That does not change anything.

OTHER TIPS

Add a using statement in order to close the connection reliably. This ensures that the connection is closed even if an exception occurs. Change your code as follows:

public static DataTable ExecuteQuery(string query, string table)
    {
        using(SqlConnection cnn = new SqlConnection(ConnectionInfo))
        {
            SqlDataAdapter Adp = new SqlDataAdapter(query, cnn);
            DataSet Ds = new DataSet();
            Adp.Fill(Ds, table);
            return Ds.Tables[table];
        }
    }

Whatever the opening/closing of the connections should be done in try-catch-finally block.

And we should not be using "using" [using (SqlConnection connection = new SqlConnection(connectionString))] block. Because if something goes wrong with the network or any exception cause. Connection is not closed. So better to be use try-catch block.

    public static DataTable ExecuteQuery(string query, string table)
    {
        DataSet Ds = new DataSet();

        SqlConnection cnn = new SqlConnection(ConnectionInfo);

        try{
            SqlDataAdapter Adp = new SqlDataAdapter(query, cnn);
            Adp.Fill(Ds, table);
            return Ds.Tables[table]; 
        }
        catch{
            throw;
        }
        finally{
            cnn.Close();
        }

    }
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top