-2

I have winform that gets id from comboBox and multiple values from listBox then I need to store this listBox items with id of my comboBox item.

**Current behavior`

If there is only one item in my listBox it stores. But if there is multiple items (which is purpose of using listBox) it return following error:

Message: Procedure or function InsertSerial has too many arguments specified.

Expected behavior

Get all items in listBox and add them to database together (bulk insert)

Code

Table schema

CREATE TABLE [dbo].[Serials] (
    [Id]        INT          IDENTITY (1, 1) NOT NULL,
    [ProductId] INT          NOT NULL,
    [Serial]    VARCHAR (50) NOT NULL,
    [Sold]      BIT          DEFAULT ((0)) NOT NULL,
    PRIMARY KEY CLUSTERED ([Id] ASC),
    CONSTRAINT [FK_Serials_Products] FOREIGN KEY ([ProductId]) REFERENCES [dbo].[Products] ([Id])
);

StoredProcedur

CREATE PROCEDURE [dbo].[InsertSerial]
    @ProductId INT,
    @Serial NVARCHAR(50),
    @Sold INT
AS
    begin
    INSERT into Serials(ProductId, Serial, Sold) values (@ProductId,@Serial,@Sold);
    end

form

// save button
private void snStoreBtn_Click(object sender, EventArgs e)
{
    try
    {
        using (SqlConnection cn = new SqlConnection(ConfigurationManager.ConnectionStrings["MyApp.Properties.Settings.MyAppDatabaseString"].ConnectionString))
        {
            if (cn.State == ConnectionState.Closed)
                cn.Open();
            using (SqlCommand cmd = new SqlCommand("dbo.InsertSerial", cn))
            {
                foreach (var item in listBox1.Items)
                {
                    Console.WriteLine("Item::: {0}", item.ToString());
                    cmd.CommandType = CommandType.StoredProcedure;
                    cmd.Parameters.AddWithValue("@ProductId", selectedProductId);
                    cmd.Parameters.AddWithValue("@Serial", item.ToString()); //Message: Procedure or function InsertSerial has too many arguments specified.
                    cmd.Parameters.AddWithValue("@Sold", "0");
                    cmd.ExecuteNonQuery();
                }
                cn.Close();
            }
            backgroundWorker1.RunWorkerAsync();
            progressBarStoringData.Visible = true;
            progressLabel.Visible = true;
        }
        // clear the listBox
        listBox1.Items.Clear();
    }
    catch (SqlException ex)
    {
        StringBuilder errorMessages = new StringBuilder();
        for (int i = 0; i < ex.Errors.Count; i++)
        {
            errorMessages.Append("Index #" + i + "\n" +
                "Message: " + ex.Errors[i].Message + "\n" +
                "LineNumber: " + ex.Errors[i].LineNumber + "\n" +
                "Source: " + ex.Errors[i].Source + "\n" +
                "Procedure: " + ex.Errors[i].Procedure + "\n");
        }
        Console.WriteLine(errorMessages.ToString());
    }
}

Question

How exactly should I loop my listBox items to be stored in database properly?

mafortis
  • 6,750
  • 23
  • 130
  • 288
  • 3
    Does this answer your question? [Sql Bulk Copy/Insert in C#](https://stackoverflow.com/questions/18841000/sql-bulk-copy-insert-in-c-sharp) and [What’s the best way to bulk database inserts from c#?](https://stackoverflow.com/questions/682015/what-s-the-best-way-to-bulk-database-inserts-from-c) and [Bulk insert strategy from c# to SQL Server](https://stackoverflow.com/questions/28664844/bulk-insert-strategy-from-c-sharp-to-sql-server) –  Aug 27 '21 at 17:06
  • 1
    In addition to suggested duplicated, you can use either a SQL Transaction to surround the loop or a single query having multiples values if applicable and the engine should do the transaction for you. –  Aug 27 '21 at 17:07

2 Answers2

3

Add the parameters only once, and just change their values for each execution.

            using (SqlCommand cmd = new SqlCommand("dbo.InsertSerial", cn))
            {
                cmd.CommandType = CommandType.StoredProcedure;
                var pProductId = cmd.Parameters.Add("@ProductId", SqlDbType.Int);
                var pSerial = cmd.Parameters.AddWithValue("@Serial", 
SqlDbType.VarChar); 
                pSerial.Size=50;
                var pSold = cmd.Parameters.AddWithValue("@Sold", SqlDbType.Int);
                foreach (var item in listBox1.Items)
                {
                    pProductId.Value = selectedProductId;
                    pSerial.Value = item.ToString();
                    pSold.Value = 0;
                    Console.WriteLine("Item::: {0}", item.ToString());
                    cmd.ExecuteNonQuery();
                }
                cn.Close();
            }
David Browne - Microsoft
  • 80,331
  • 6
  • 39
  • 67
-1

Put using (SqlCommand cmd = new SqlCommand("dbo.InsertSerial", cn)) inside the loop, so you make a new command each time, otherwise you're adding more params to the same command each time.

Ian
  • 303
  • 3
  • 5
  • it says `ExecuteNonQuery requires an open and available Connection. The connection's current state is closed.'` refer to `cmd.ExecuteNonQuery();` – mafortis Aug 27 '21 at 17:04
  • The cn.Close() needed to stay outside. I'll leave it, as the above answer does the job in your case. In more complex scenarios it's safer to make a new command. – Ian Aug 27 '21 at 17:11