The delete button is too slow what is the problem

Question

I use this code to delete records which is selected by checkbox in the datagridview, bout it take too long time to do the command

private void delete_Click(object sender, EventArgs e)
    {
        foreach(DataGridViewRow item in advancedDataGridView1.Rows)
        {
            if(bool.Parse(item.Cells[0].Value.ToString()))
            {

                conn.Open();
                SqlCommand cmd = new SqlCommand("delete from tabl where id = '" + item.Cells[1].Value.ToString() + "'", conn);
                cmd.ExecuteNonQuery();
                conn.Close();
            }
        }
        MessageBox.Show("Successfully Deleted....");
    }

and I use this code for the checkbox

private void Chkselectall_CheckedChanged(object sender, EventArgs e)
    {
        for(int n = 0; n< advancedDataGridView1.Rows.Count;n++)
        {
            advancedDataGridView1.Rows[n].Cells[0].Value = chkselectall.Checked;
        }
    }

What should I do the solve this problem?

Answer 1

Well, you execute a bunch of commands in a sequence. For each one, you open a new connection, close it, and fast as it is, there is always an overhead involved. You better get a list of IDs you need to delete and change your command to

delete from tabl where id in (…)

Answer 2

Few things that you are doing in wrong way.

• You are using inline queries, we are already having many threads(in SO)that discussing about this issue, as it opens a wide door for SQLInjection.
• The reason that slow down the performance of the execution is the way you are processing. ie., you are iterating through each row and for each matching items you are opening a connection and executing the delete query.

I prefer you to use the following steps:

• Iterate through rows and collect the items to delete based on the condition.
• construct a prameterized query which accept a parameter of type string.
• Assign the collected ids as the value for the parameter.
• Execute the query.

Please find the sample code below:

private void delete_Click(object sender, EventArgs e)
{
    List<string> selectedIds = new List<string>();
    foreach (DataGridViewRow item in advancedDataGridView1.Rows)
    {
        if (bool.Parse(item.Cells[0].Value.ToString()))
        {
            selectedIds.Add("'" + item.Cells[1].Value.ToString() + "'");
            // collecting all ids
        }
    }
    String sql = "delete from tabl where id in(@idsToDelete)";
    using (SqlConnection cn = new SqlConnection("Your connection string here")) 
    {
        cn.Open();
        using (SqlCommand cmd = new SqlCommand(sql, cn)) 
        {
            cmd.Parameters.Add("@idsToDelete", SqlDbType.VarChar).Value = string.Join(",", selectedIds);
            cmd.ExecuteNonQuery();
        }
    }      
}

Answer 3

thx all for your help i found what i need to and i hop it help anyone else need something like that

String sql;
int parameterCounter;
SqlParameter parameter;

private void delete_Click(object sender, EventArgs e)
{
    sql = "delete from tabl where id in (";
    parameterCounter = 0;

    using (SqlConnection cn = new SqlConnection("....")) {
    using (SqlCommand cmd = new SqlCommand(sql, cn)) {
    foreach (DataGridViewRow item in advancedDataGridView1.Rows) {
     if (bool.Parse(item.Cells[0].Value.ToString())) {
        parameterCounter++;
        parameter = new SqlParameter();
        parameter.ParameterName = "@par" + parameterCounter.ToString();
        parameter.DbType = System.Data.DbType.Int32;
        parameter.Value = item.Cells[1].Value;
        cmd.Parameters.Add(parameter);
        sql = sql + $"{parameter.ParameterName},";
        // collecting all ids
     }
  }
  sql = sql.TrimEnd(',');
  sql = sql + ")";

  cmd.CommandText = sql;
  cmd.Connection = cn;
  cn.Open();
  cmd.ExecuteNonQuery();
    MessageBox.Show("Successfully Deleted....");
}

Answer 4

You should put the sql operations out of the foreach loop. Concat the sql string in the loop and execute it out of the loop.

private void delete_Click(object sender, EventArgs e)
    {
        StringBuilder sb = new StringBuilder();
        foreach(DataGridViewRow item in advancedDataGridView1.Rows)
        {
            if(bool.Parse(item.Cells[0].Value.ToString()))
            {
                sb.AppendFormat("delete from tabl where id='{0}';{1}", item.Cells[1].Value, Environment.NewLine);

            }
        }
                conn.Open();
                SqlCommand cmd = new SqlCommand(sb.ToString(), conn);
                cmd.ExecuteNonQuery();
                conn.Close();
        MessageBox.Show("Successfully Deleted....");
    }

Theoretically, passing sql parameters directly is quite dangerous because of sql injection. You have to deal with it yourself