How to solve the "Connection Was Not Closed, The connections current state is open" in c#?-CodePudding

So I'm trying to practice c# and stumbled with the connection error, I already stated that my connection will be closed but it tells me that my connection is still open. I really have no idea what's wrong with this.

 public void getdept()
    {
        con.Open();
        string query = "SELECT * FROM positions where PositionName="   cbxposname.SelectedValue.ToString()   "";
        SqlCommand cmd = new SqlCommand(query, con);
        DataTable dt = new DataTable();
        SqlDataAdapter sda = new SqlDataAdapter(query, con);
        sda.Fill(dt);
       
        foreach (DataRow dr in dt.Rows)
        {
            txtdeptname.Text = dr["Department"].ToString();

        }
        
        con.Close();


    }

Any tips is welcomed!

CodePudding user response：

You appear to be using a common connection object. Don't. Create your connection object where you use. Do so with a using statement and then the connection will be closed and destroyed at the end of the block. Store your connection string in a common location and then use that each time to create a new connection, e.g.

var table = new DataTable();

using (var connection = new SqlConnection(connectionString)
using (var adapter = new SqlDataAdapter("SQL query here", connection)
{
    adapter.Fill(table);
}

// use table here.

There are a number of things to note from this code, other than the using block.

Firstly, it doesn't explicitly open the connection because there's no point. The Fill and Update methods of a data adapter will implicitly open the connection if it's currently closed and it will implicitly close the connection if it opened it. When using a data adapter, the only reason to open the connection explicitly is if you're calling multiple Fill and/or Update methods, so the connection is closed and reopened in between. Even if you do open the connection though, there's still no need to close it explicitly because that happens implicitly at the end of the using block.

Secondly, this code doesn't create a command object because there's no point. In your original code, you create a command object and then you don't use it. If you already have a command object then you can pass that to the data adapter constructor but you don't do that. You pass the SQL query and the connection, so the data adapter will create its own SelectCommand.

In actual fact, there's no point even creating a connection object here. The data adapter has a constructor that accepts a SQL query and a connection string, so you can just create the data adapter and let it do the rest internally:

var table = new DataTable();

using (var adapter = new SqlDataAdapter("SQL query here", connectionString)
{
    adapter.Fill(table);
}

// use table here.

CodePudding user response：

You are unnecessary opening and closing connections in your case. It's not needed here.

Your code should look like this.

using (SqlConnection con = new SqlConnection(connetionString))
{
    using (DataTable dt = new DataTable())
    {
        using (SqlDataAdapter sda = new SqlDataAdapter(sql, con))
        {
            sda.Fill(dt);

            foreach (DataRow dr in dt.Rows)
            {
                txtdeptname.Text = dr["Department"].ToString();
            }
        }

    }

}

A few suggestions also, Please don't use * in the query, instead, use column names, Please use NOLOCK in the Query if it's required in your case and use the parameterized query.