Connection was not closed. Connection's current state is open

497 views Asked by At

It gives the error connection was not closed. Connection's current state is open. Please help out with the code.

  private void comboBox1_SelectedIndexChanged(object sender, EventArgs e)
    {
        SqlConnection con = new SqlConnection(@"Data Source=.\SQLEXPRESS;AttachDbFilename=C:\Users\vicky\Desktop\Gym management system\Fitness_club\vicky.mdf;Integrated Security=True;Connect Timeout=30;User Instance=True");
        try
        {
            con.Open();

            SqlCommand cmd = new SqlCommand("Select * FROM [plan] where plantype='" + comboBox1.Text + "'", con);

            SqlDataReader dr = cmd.ExecuteReader();

            while (dr.Read())
            {
                string amount = dr.GetString(1);
                textBox5.Text = amount;

            }
            con.Close();
        }

        catch(Exception ex)
        {
            MessageBox.Show(ex.Message);
        }

}
2

There are 2 answers

3
Ron Beyer On BEST ANSWER

You should be using using blocks to help with managing your objects.

    private void comboBox1_SelectedIndexChanged(object sender, EventArgs e)
    {
        string connStr = @"Data Source=.\SQLEXPRESS;AttachDbFilename=C:\Users\vicky\Desktop\Gym management system\Fitness_club\vicky.mdf;Integrated Security=True;Connect Timeout=30;User Instance=True";
        string cmdText = "Select * FROM [plan] where plantype=@planType";

        using (SqlConnection con = new SqlConnection(connStr))
        using (SqlCommand cmd = con.CreateCommand())
        {
            con.Open();
            cmd.CommandText = cmdText;
            cmd.Parameters.AddWithValue("@planType", comboBox1.Text);

            var reader = cmd.ExecuteReader(CommandBehavior.SingleRow);

            if (reader.Read())
            {
                string amount = reader.GetString(1);
                textbox5.Text = amount;
            }
        }
    }

Also note the use of parameterized queries to avoid SQL injection attacks. Since you are probably only expecting one value to be returned, you should specify the name of the column in the query and use ExecuteScalar instead of the reader and the while loop. The other alternative is to use CommandBehavior.SingleRow as the parameter to the command, which tells the command to just return a single row result.

You also have a cross-threading problem here, and you can solve it by using some invoking.

string amount = string.Empty;
if (reader.Read())
{
    amount = reader.GetString(1);
}
if (this.InvokeRequired)
    this.Invoke((MethodInvoker) delegate { textbox5.Text = amount; });
else
    textbox5.Text = amount;

Another thing to note, is to give your controls meaningful names. Its a lot easier to debug or understand a control named cbx_PlanType than combobox1, or tbx_PlanAmount rather than textbox5.

0
Bogdan Iurcu On
private void comboBox1_SelectedIndexChanged(object sender, EventArgs e)
{
    SqlConnection con = new SqlConnection(@"Data Source=.\SQLEXPRESS;AttachDbFilename=C:\Users\vicky\Desktop\Gym management system\Fitness_club\vicky.mdf;Integrated Security=True;Connect Timeout=30;User Instance=True");
    try
    {
        con.Open();

        SqlCommand cmd = new SqlCommand("Select * FROM [plan] where plantype='" + comboBox1.Text + "'", con);

        SqlDataReader dr = cmd.ExecuteReader();

        while (dr.Read())
        {
            string amount = dr.GetString(1);
            textBox5.Text = amount;

        }
        // check if connection is open
        if (con.State == 1)
            con.Close();
    }
    catch(Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
    finally
    {
        // check if connection is open
        if (con.State == 1)
            con.Close();
    }
}