Should I call Parameters.Clear when reusing a SqlCommand with a transation?

Question

I'm coding a transaction manually in ADO.NET. The example I'm working from reuses the SqlCommand which seem like a fine idea.

However, I have added parameters to my command.

My question is: in the following code, is command.Parameters.Clear() correct? Or am I doing it wrong?

using (var connection = new SqlConnection(EomAppCommon.EomAppSettings.ConnStr))
{
    connection.Open();
    SqlTransaction transaction = connection.BeginTransaction();
    SqlCommand command = connection.CreateCommand();
    command.Transaction = transaction;
    try
    {
        foreach (var itemIDs in this.SelectedItemIds)
        {
            command.CommandText = "UPDATE Item SET payment_method_id = @batchID WHERE id in (@itemIDs)";
            // IS THE FOLLOWING CORRECT?
            command.Parameters.Clear();

            command.Parameters.Add(new SqlParameter("@batchID", batchID));
            command.Parameters.Add(new SqlParameter("@itemIDs", itemIDs));
            command.ExecuteNonQuery();
        }
        transaction.Commit();
    }
    catch (Exception ex)
    {
        MessageBox.Show("Failed to update payment batches, rolling back." + ex.Message);
        try
        {
            transaction.Rollback();
        }
        catch (Exception exRollback)
        {
            if (!(exRollback is InvalidOperationException)) // connection closed or transaction already rolled back on the server.
            {
                MessageBox.Show("Failed to roll back. " + exRollback.Message);
            }
        }
    }
}

Why not just create the command inside the loop? Or create an SQL statement that can do all of the updates in one go, and avoid the loop entirely? — Damien_The_Unbeliever, Jan 17 '13 at 08:11

score 13 · Accepted Answer · edited May 23 '17 at 12:33

13

Since you're repeatedly executing the same query, it's unnecessary to clear them - you can add the parameters outside the loop and just fill them inside.

try
{
    command.CommandText = "UPDATE Item SET payment_method_id = @batchID WHERE id in (@itemIDs)";
    command.Parameters.Add(new SqlParameter("@batchID", 0));
    command.Parameters.Add(new SqlParameter("@itemIDs", ""));

    foreach (var itemIDs in this.SelectedItemIds)
    {
        command.Parameters["@batchID"].Value = batchID;
        command.Parameters["@itemIDs"].Value = itemIDs;
        command.ExecuteNonQuery();
    }
    transaction.Commit();
}

Note - you can't use parameters with IN as you've got here - it won't work.

edited May 23 '17 at 12:33

Community

1
1

answered Jan 17 '13 at 08:10

Richard

29,854
11
77
120

yeah I just discovered that about the IN clause, thanks for the link – Aaron Anodide Jan 17 '13 at 08:16
Besides the additional processing, is there any other downside to clearing and re-adding them? Not clearing them also makes the code less DRY. – Lukas Oct 22 '20 at 15:35

score 1 · Answer 2 · answered Jan 17 '13 at 08:08

1

In this condition you need it as you need set new parameters values, so its correct.

By the way, move

command.CommandText = ".."

outside of the loop too, as it's never changed.

answered Jan 17 '13 at 08:08

Tigran

61,654
8
86
123

Should I call Parameters.Clear when reusing a SqlCommand with a transation?

2 Answers2

Linked