0 
private void comT_SelectedIndexChanged(object sender, EventArgs e)
    {
        if (comT.SelectedIndex != -1)
        {
            SqlCommand cmd = new SqlCommand(@"SELECT ISNULL(substring(MAX(tCode),3,2),'00')
            FROM Teacher
            WHERE dCode = '" + comT.SelectedValue.ToString() + "'", MUControlClass.con);
            SqlDataReader dr = cmd.ExecuteReader();
            dr.Read();  <--HERE IS ERROR **"ExecuteReader requires an open and available Connection. The connection's current state is closed."**
            if (dr[0].ToString() != "00")
            {
                int dugaar = Convert.ToInt32(dr[0]) + 1;
                if (dugaar > 9)
                {
                    msk.Text = comT.SelectedValue.ToString() + dugaar.ToString();
                }
                else
                    msk.Text = comT.SelectedValue.ToString() + "0" + dugaar.ToString();
            }
            else
            {
                msk.Text = comT.SelectedValue.ToString() + "01";
            }
            dr.Close();
        }
    }

ExecuteReader requires an open and available Connection. The connection's current state is closed. Error

Improve this question
3
  • 2
    Error message is pretty clear. MUControlClass.con is not open. Commented May 14, 2020 at 12:21
  • You should probably use using {...} blocks for your query calls, and always create a new connection when doing so. See in a “using” block is a SqlConnection closed on return or exception? for an example. Commented May 14, 2020 at 12:25
  • This looks suspiciously like you're trying to share a single connection object across multiple methods. This is very often the wrong thing to do. Share the connection string but not the connection object. Create the object close to usage instead. Commented May 14, 2020 at 12:25

3 Answers 3

Reset to default
3

The immediate error is that the connection is not open, as it told you; so ... open it; however, there are a lot of other serious problems here - most notably "SQL injection", but also non-disposed objects. You can fix SQL injection by using parameters, and the non-disposed problem with lots of using, but: I strongly suggest you make use of tools that will help you get this right by default. For example:

private void comT_SelectedIndexChanged(object sender, EventArgs e)
{
    if (comT.SelectedIndex != -1)
    {
        using (var conn = new SqlConnection(yourConnectionString))
        {
            string max = conn.QuerySingle<string>(@"
SELECT ISNULL(substring(MAX(tCode),3,2),'00')
FROM Teacher
WHERE dCode = @dCode", new { dCode = comT.SelectedValue.ToString() });
            if (max != null)
            {
                // your parse/etc logic
            }
        }
    }
}

This is:

  • moving our connection lifetime to be local to this method, which will stop a lot of connection usage problems
  • using "Dapper" to provide the QuerySingle<T> code
  • which means you don't need to mess with commands or readers
  • adding parameter usage via @dCode and the new { dCode = ... } usage

Note it might look like we've still not opened the connection here, but Dapper is happy to do that for us; if it is given a closed connection, it opens the connection for the duration of the query only.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

One of the reasons I use Stored Procedures. It forces the developper to use parameters.
@jscarle I've seen people do cmd.CommandText = "EXEC myProc @id=" + id + ", @name='" + name + "'"; - never under-estimate the ability to screw up; broadly (but not exactly) related: blog.marcgravell.com/2017/12/…
@jscarle Not to mention stored procedures that takes in parameters but use it inside to create dynamic SQL statements using string concatenations.... Here's an example
1

Wherever the connection is created in MUControlClass you need to call con.Open().

Improve this answer

Comments

1

Your connection is not opened.

Simplified flow of connection and interaction with database is like this:

using(SqlConnection con = new SqlConnection(yourConnectionString))
{
    con.Open() // You do not open this one so it returns that error
    using(SqlCommand cmd = new SqlCommand(yourSqlCommand, con))
    {
        using(SqlDataReader dr = cmd.ExecuteReader())
        {
            if(dr.Read())
            {
                // Do something
            }
        }
    }
}

We do not need to do close/destroy anything as long as it is wrapped in using since they all implements IDisposable

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.