The connection was not closed. The connection's current state is open

Question

Help! it says The connection was not closed. The connection's current state is ope but i did close it , pls help ? thank you in advance

private void inserttransaction()
    {

        for (int i = 0; i < dataPOS.Rows.Count; i++)
        {
            con.Open();
            dataPOS.Rows[i].Selected = true;
            cmd = new SqlCommand(@"INSERT INTO TRANSACTIONS (TransactionCode,TransactionDate,ItemCode,ItemName,Quantity,Price,Total)
                                 VALUES
                                    ('"+ dataPOS.SelectedRows[0].Cells[0].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[1].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[2].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[3].Value.ToString()+"' , '"+ dataPOS.SelectedRows[0].Cells[4].Value.ToString() +"' '"+ dataPOS.SelectedRows[0].Cells[5].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[6].Value.ToString() +"')", con);
            cmd.ExecuteNonQuery();
            dataPOS.Rows[i].Selected = false;
            con.Close();
        }

    }

You should wrap your connection in a `using` block so that it takes care of the closing. What would happen if your `ExecuteNonQuery` threw an exception? — Steve, Mar 27 '17 at 14:14
You need to use SqlParameters with your SqlCommand to avoid SQL Injection. SqlCommand should be within a using() block. Open the connection once (if its not already open) *outside* the loop, loop, then close. — Alex K., Mar 27 '17 at 14:14
using (SqlConnection connection = new SqlConnection(connectionString)) { //put your BL here. } — Dutt93, Mar 27 '17 at 14:18

score 3 · Accepted Answer · answered Mar 27 '17 at 14:15

You can't open a connection which is already open.

con.Open();

for (int i = 0; i < dataPOS.Rows.Count; i++)
{
    dataPOS.Rows[i].Selected = true;
    cmd = new SqlCommand(@"INSERT INTO TRANSACTIONS (TransactionCode,TransactionDate,ItemCode,ItemName,Quantity,Price,Total)
                         VALUES
                            ('"+ dataPOS.SelectedRows[0].Cells[0].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[1].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[2].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[3].Value.ToString()+"' , '"+ dataPOS.SelectedRows[0].Cells[4].Value.ToString() +"' '"+ dataPOS.SelectedRows[0].Cells[5].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[6].Value.ToString() +"')", con);
    cmd.ExecuteNonQuery();
    dataPOS.Rows[i].Selected = false;
}

con.Close();

But I'd use the using-statement anyway:

using(var con = new SqlConnection("connection-string.."))
{
    con.Open();

    for (int i = 0; i < dataPOS.Rows.Count; i++)
    {
        dataPOS.Rows[i].Selected = true;
        cmd = new SqlCommand(@"INSERT INTO TRANSACTIONS (TransactionCode,TransactionDate,ItemCode,ItemName,Quantity,Price,Total)
                             VALUES
                                ('"+ dataPOS.SelectedRows[0].Cells[0].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[1].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[2].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[3].Value.ToString()+"' , '"+ dataPOS.SelectedRows[0].Cells[4].Value.ToString() +"' '"+ dataPOS.SelectedRows[0].Cells[5].Value.ToString() +"' , '"+ dataPOS.SelectedRows[0].Cells[6].Value.ToString() +"')", con);
        cmd.ExecuteNonQuery();
        dataPOS.Rows[i].Selected = false;
    }
}  // con.Close() not necessary

You should also use parameterized queries instead of concatenating strings to build your sql query. Otherwise you are vulnerable to sql-injection attacks.

in the connection string ? like this new SqlConnection(@"server=localhost\SqlExpress;Initial Catalog = CORNERFLAG;Integrated Security = true;"); — Pudge, Mar 27 '17 at 14:19
it works but there another problem , its says that Cannot insert explicit value for identity column in table 'TRANSACTIONS' when IDENTITY_INSERT is set to OFF. ? — Pudge, Mar 27 '17 at 14:29
@Pudge: maybe `TransactionCode` is your primary key and this column is an `IDENTITY` column, which means it's value is auto-generated on inserts. Then you can't insert this value programmatically. You either can't insert insert it or you have to disable it temporarily: `SET IDENTITY_INSERT tablename ON .... your query ... SET IDENTITY_INSERT tablename OFF` — Tim Schmelter, Mar 27 '17 at 14:34
@Pudge: sure, just change the [`Identity` Specification](https://learn.microsoft.com/en-us/sql/relational-databases/tables/table-column-properties-sql-server-management-studio) `Is Identity` to `false`. — Tim Schmelter, Mar 27 '17 at 14:38
i removed the TransactionCode and it works but it is not perfect , but thank you sir , for your solution and time , thank you so much :) — Pudge, Mar 27 '17 at 14:38

daniell89 · Answer 2 · 2017-03-27T14:38:11.590

Try to add using clause and open conn before looping. And I think you should consider adding SqlParameter insted of adding data direct from your source collection (sql injection risk).

            using (var con = new SqlConnection("your conn string"))
            {
                con.Open();
                SqlCommand cmd = new SqlCommand();
                cmd.CommandType = CommandType.Text;
                cmd.Connection = con;

                List<SqlParameter> sqlParams = new List<SqlParameter>();
                sqlParams.Add(new SqlParameter("@param0", null));
                sqlParams.Add(new SqlParameter("@param1", null));
                sqlParams.Add(new SqlParameter("@param2", null));
                sqlParams.Add(new SqlParameter("@param3", null));
                sqlParams.Add(new SqlParameter("@param4", null));
                sqlParams.Add(new SqlParameter("@param5", null));
                sqlParams.Add(new SqlParameter("@param6", null));

                cmd.Parameters.AddRange(sqlParams.ToArray());

                for (int i = 0; i < dataPOS.Rows.Count; i++)
                {
                    cmd.Parameters["@param0"].Value = dataPOS.SelectedRows[0].Cells[0].Value.ToString();
                    cmd.Parameters["@param1"].Value = dataPOS.SelectedRows[0].Cells[1].Value.ToString();
                    cmd.Parameters["@param2"].Value = dataPOS.SelectedRows[0].Cells[2].Value.ToString();
                    cmd.Parameters["@param3"].Value = dataPOS.SelectedRows[0].Cells[3].Value.ToString();
                    cmd.Parameters["@param4"].Value = dataPOS.SelectedRows[0].Cells[4].Value.ToString();
                    cmd.Parameters["@param5"].Value = dataPOS.SelectedRows[0].Cells[5].Value.ToString();
                    cmd.Parameters["@param6"].Value = dataPOS.SelectedRows[0].Cells[6].Value.ToString();

                    dataPOS.Rows[i].Selected = true;
                    cmd.CommandText = @"INSERT INTO TRANSACTIONS (TransactionCode,TransactionDate,ItemCode,ItemName,Quantity,Price,Total)
                                        VALUES (@param0, @param1, @param2, @param3, @param4, @param5,@param6 )";
                    cmd.ExecuteNonQuery();
                    dataPOS.Rows[i].Selected = false;
                }
            }

The connection was not closed. The connection's current state is open

2 Answers2