Procedure or function SP_InsertBookDetail has too many arguments specified

Question

I'm trying to save datagridview data to a database table.

SP_InsertBookHeader worked fine but when executing SP_InsertBookDetail, I get an error:

Procedure or function SP_InsertBookDetail has too many arguments specified

I checked procedure and C# code; both parameters are same.

Book.cs:

private void addRecord()
{
        try
        {
            string btnClick = CusMsgBox.showBox("SAVE", "Do you want to save record?");

            if (btnClick == "1")
            {
                var bookInfo = new BookInformation();

                bookInfo.BookID = txtBookID.Text;
                bookInfo.Copies = int.Parse(txtCopies.Text);
                bookInfo.Date = DateTime.Parse(dtpDate.Value.ToString());

                var bookList = new List<BookInformation>();

                foreach (DataGridViewRow items in dgvBook.Rows)
                {
                    var bookDetailInfo = new BookInformation();

                    bookDetailInfo.BookID = txtBookID.Text;
                    bookDetailInfo.ISBN = items.Cells["ISBN"].Value.ToString();
                    bookDetailInfo.Name = items.Cells["Name"].Value.ToString();
                    bookDetailInfo.Description = items.Cells["Description"].Value.ToString();
                    bookDetailInfo.PublishDate = DateTime.Parse(items.Cells["Publish Date"].Value.ToString());
                    bookDetailInfo.Language = items.Cells["Language"].Value.ToString();
                    bookDetailInfo.AuthorID = items.Cells["Author"].Value.ToString();
                    bookDetailInfo.Category = items.Cells["Category"].Value.ToString();
                    bookDetailInfo.Edition = int.Parse(items.Cells["Edition"].Value.ToString());
                    bookDetailInfo.Price = decimal.Parse(items.Cells["Price"].Value.ToString());

                    bookList.Add(bookDetailInfo);
                }

                var bookService = new BookService();

                int line = bookService.insertBook(bookInfo, bookList);

                if (line > 0)
                {
                    clearRecord();
                    clearBookRecord();
                    viewBookID();
                    viewBookRecord();
                    CusNotifi.showSuccess("Data Inserted Succesfully!");
                }
                else
                {
                    CusNotifi.showError("Data Insert Failed!");
                }
            }
            else
            {
                CusMsgBox cmb = new CusMsgBox();
                cmb.Dispose();
            }
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.ToString(), "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
        }
}

BookService.cs:

public int insertBook(BookInformation bookInfo, List<BookInformation> bookList)
{
    var bookData = new BookData();
    return bookData.addRecord(bookInfo, bookList);
}

BookData.cs

public int addRecord(BookInformation bookInfo, List<BookInformation> bookList)
{
    try
    {
        cmd = new SqlCommand("SP_InsertBookHeader", dbcon.ActiveCon());
        sda = new SqlDataAdapter(cmd);

        sda.SelectCommand.CommandType = CommandType.StoredProcedure;  
        sda.SelectCommand.Parameters.Add("@BookID", SqlDbType.VarChar).Value = bookInfo.BookID;
        sda.SelectCommand.Parameters.Add("@Copies", SqlDbType.Int).Value = bookInfo.Copies;
        sda.SelectCommand.Parameters.Add("@Date", SqlDbType.DateTime).Value = bookInfo.Date;

        var cmdDetail = new SqlCommand("SP_InsertBookDetail", dbcon.ActiveCon());
        var sdaDetail = new SqlDataAdapter(cmdDetail);
        sdaDetail.SelectCommand.CommandType = CommandType.StoredProcedure;

        foreach (BookInformation items in bookList)
        {                       
            sdaDetail.SelectCommand.Parameters.Add("@BookID", SqlDbType.VarChar).Value = items.BookID;
            sdaDetail.SelectCommand.Parameters.Add("@ISBN", SqlDbType.VarChar).Value = items.ISBN;
            sdaDetail.SelectCommand.Parameters.Add("@Name", SqlDbType.VarChar).Value = items.Name;
            sdaDetail.SelectCommand.Parameters.Add("@Description", SqlDbType.VarChar).Value = items.Description;
            sdaDetail.SelectCommand.Parameters.Add("@PublishDate", SqlDbType.DateTime).Value = items.PublishDate;
            sdaDetail.SelectCommand.Parameters.Add("@Language", SqlDbType.VarChar).Value = items.Language;
            sdaDetail.SelectCommand.Parameters.Add("@AuthorID", SqlDbType.VarChar).Value = items.AuthorID;
            sdaDetail.SelectCommand.Parameters.Add("@Category", SqlDbType.VarChar).Value = items.Category;
            sdaDetail.SelectCommand.Parameters.Add("@Edition", SqlDbType.Int).Value = items.Edition;
            sdaDetail.SelectCommand.Parameters.Add("@Price", SqlDbType.Decimal).Value = items.Price;
        }

        int line = cmd.ExecuteNonQuery();
        int line1 = cmdDetail.ExecuteNonQuery();

        return line;
    }
    catch
    {
        throw;
    }
    finally
    {
        cmd.Dispose();
        dbcon.CloseCon();
    }
}

Procedure:

ALTER PROCEDURE [dbo].[SP_InsertBookDetail]
    @BookID VARCHAR(20),
    @ISBN VARCHAR(50),
    @Name VARCHAR(100),
    @Description VARCHAR(10),
    @PublishDate DATETIME,
    @Language VARCHAR(20),
    @AuthorID VARCHAR(20),
    @Category VARCHAR(20),
    @Edition INT,
    @Price DECIMAL(18, 2)
AS
BEGIN
    INSERT INTO Book_Detail(BookID, ISBN, Name, Description, PublishDate, Language, AuthorID, Category, Edition, Price) 
    VALUES (@BookID, @ISBN, @Name, @Description, @PublishDate, @Language, @AuthorID, @Category, @Edition, @Price)
END

Why are you not inserting the command inside the ForEach? It seems like you could be adding the same parameter multiple times if you have multiple books, yet the stored procedure is only expecting 1 of each parameter. — JohnPete22, Oct 16 '19 at 17:22
Welcome to Stackoverflow. Please read [How to ask](https://stackoverflow.com/help/how-to-ask) on how to ask a good question. You posted a lot of code but the problem presumably narrows down to only a few lines of code. You help others a lot understanding your question and solve your problem if you shorten the code to relevant lines only. — Jonathan, Oct 16 '19 at 17:22
To me, it seems the ForEach loop should be instantiating it own SQL command and connection, and insert there. Or you need to modify your stored procedure to take an array of items, but that is going to get messy. — JohnPete22, Oct 16 '19 at 17:24
Note that the `sp_` prefix is reserved by Microsoft and should be avoided (it means Special Procedure). Using the prefix *can* have performance impacts, or even result in your SP suddenly not working: [Is the sp_ prefix still a no-no?](https://sqlperformance.com/2012/10/t-sql-queries/sp_prefix) — Thom A, Oct 16 '19 at 18:04
On a different note, well done for using `Parameters.Add`. It is a refreshing and pleasant sight to see someone parametrising their code so well. — Thom A, Oct 16 '19 at 18:05
Shouldn't the execution of `SP_InsertBookDetail` be *inside* the `ForEach`? Otherwise wouldn't this add multiple instances of the same parameter to the execution statement? You should also have those connections inside a `Using`, so that the are gracefully disposed of, before the next statement is executed (in the loop). Alternatively, you could use a table-value parameter and pass the whole lot in one go. [Passing a Table-Valued Parameter to a Stored Procedure](https://learn.microsoft.com/en-us/dotnet/framework/data/adonet/sql/table-valued-parameters#passing) — Thom A, Oct 16 '19 at 18:08
I would also suggest you utilize the USING clause for an IDisposable objects (like connections and commands). Then you can stop using the try/catch/finally blocks. Catching exceptions are a performance hole that are not needed if you do nothing with them other then throw them again. The USING clause wraps all that quite nicely and ensures your objects are closed and disposed properly. — Sean Lange, Oct 16 '19 at 18:14
As it is coded you keep adding more and more parameters. You need to either change the value of the parameters on each iteration after the first one, or simply add Parameters.Clear() to the beginning of your loop so it wipes out the existing parameters. And you need to execute the command INSIDE the loop, not outside. — Sean Lange, Oct 16 '19 at 18:17
My C# is awful at best, but is it me, or is the OP relying on an already active connection for their command, @SeanLange? `cmd = new SqlCommand("SP_InsertBookHeader", dbcon.ActiveCon());` — Thom A, Oct 16 '19 at 18:18
@Larnu yes the command object is defined outside of this code which is cringe worthy for sure. There are many other major problems with this code. — Sean Lange, Oct 16 '19 at 18:23

score 2 · Accepted Answer · answered Oct 16 '19 at 18:46

Not totally sure what you are trying to do here but you have a number of things in your code that are a bit left of center.

A couple of things you don't want to do.

use a connection that is opened from somewhere else.
Leave varchar parameters the default length

I would restructure this method to something more along these lines.

public int addRecord(BookInformation bookInfo, List<BookInformation> bookList)
        {
            try
            {
                using (SqlConnection conn = new SqlConnection("Your Connection String Here"))
                {
                    conn.Open();
                    using (SqlCommand cmd = new SqlCommand("SP_InsertBookHeader", conn))
                    {
                        cmd.CommandType = CommandType.StoredProcedure;
                        cmd.Parameters.Add("@BookID", SqlDbType.VarChar).Value = bookInfo.BookID;
                        cmd.Parameters.Add("@Copies", SqlDbType.Int).Value = bookInfo.Copies;
                        cmd.Parameters.Add("@Date", SqlDbType.DateTime).Value = bookInfo.Date;
                        cmd.ExecuteNonQuery();
                    }

                    using (SqlCommand cmdLine = new SqlCommand("SP_InsertBookDetail", conn))
                    {
                        cmdLine.CommandType = CommandType.StoredProcedure;
                        foreach (BookInformation items in bookList)
                        {
                            cmdLine.Parameters.Clear();
                            cmdLine.Parameters.Add("@BookID", SqlDbType.VarChar, 20).Value = items.BookID;
                            cmdLine.Parameters.Add("@ISBN", SqlDbType.VarChar, 50).Value = items.ISBN;
                            cmdLine.Parameters.Add("@Name", SqlDbType.VarChar, 100).Value = items.Name;
                            cmdLine.Parameters.Add("@Description", SqlDbType.VarChar, 10).Value = items.Description;
                            cmdLine.Parameters.Add("@PublishDate", SqlDbType.DateTime).Value = items.PublishDate;
                            cmdLine.Parameters.Add("@Language", SqlDbType.VarChar, 20).Value = items.Language;
                            cmdLine.Parameters.Add("@AuthorID", SqlDbType.VarChar, 20).Value = items.AuthorID;
                            cmdLine.Parameters.Add("@Category", SqlDbType.VarChar, 20).Value = items.Category;
                            cmdLine.Parameters.Add("@Edition", SqlDbType.Int).Value = items.Edition;
                            cmdLine.Parameters.Add("@Price", SqlDbType.Decimal, 18).Value = items.Price;
                            cmdLine.ExecuteNonQuery();
                        }
                    }
                    return bookList.Count; //not really sure what you want to return here
                }
            }
            catch (Exception ex)
            {
                //Do something with your exceptions here.
                //Log them in the database or an exception log
                //report this back to the calling application or the user
            }
        }

im returning ExecuteNonQuery value to book.cs, in book.cs int line = bookService.insertBook(bookInfo, bookList) and if line>0 message will pop up that data insert successfully — Kasun Subhashana, Oct 17 '19 at 15:41
The return from a stored procedure indicates the status of the execution. Is that was you are looking for? I thought maybe you wanted to return the identity value of the newly inserted row or maybe the count of books. — Sean Lange, Oct 17 '19 at 15:43
im using app.cofing to connection string and i created .dll to open-dbcon.ActiveCon() and close connection. so is it a bad??? — Kasun Subhashana, Oct 17 '19 at 15:49
Sort of...using the config file for your connection string is great. But creating another object to manage the connection is not great. The point of the USING clause is that it will close and dispose of your objects automatically )even if there is an exception) when the code block exits. It is much safer, and easier. Not to mention a lot cleaner. Notice in the code sample here I don't close or dispose of anything? That is because the IDisposable interface has code to handle all of this. That interface calls the methods defined in the objects when appropriate. It handles connections cleanly. — Sean Lange, Oct 17 '19 at 15:56

Thom A · Answer 2 · 2019-10-16T18:46:20.133

Someone please correct me/this is this is wrong. C# is far from my forté and I'm almost entirely basing this answer off of the document and this answer.

private void addRecord()
    {
        try
        {
            string btnClick = CusMsgBox.showBox("SAVE", "Do you want to save record?");

            if (btnClick == "1")
            {

                Using (SqlConnection conn = new SqlConnection(connectionString)) //No idea where your connection string is
                {
                    Using (SqlCommand cmd = new SqlCommand("SP_InsertBookHeader", conn))
                    {
                        cmd.CommandType = CommandType.StoredProcedure;
                        cmd.Parameters.Add("@BookID", SqlDbType.VarChar).Value = txtBookID.Text;
                        cmd.Parameters.Add("@Copies", SqlDbType.Int).Value = int.Parse(txtCopies.Text);
                        cmd.Parameters.Add("@Date", SqlDbType.DateTime).Value = DateTime.Parse(dtpDate.Value.ToString());

                        conn.Open();
                        int line = cmd.ExecuteNonQuery();
                    }
                }
                foreach (DataGridViewRow items in dgvBook.Rows)
                {
                    Using (SqlConnection conn = new SqlConnection(connectionString))    
                    {

                        Using (SqlCommand cmd = new SqlCommand("SP_InsertBookDetail", conn))
                        {
                            cmd.CommandType = CommandType.StoredProcedure;
                            cmd.Parameters.Add("@BookID", SqlDbType.VarChar).Value = txtBookID.Text;
                            cmd.Parameters.Add("@ISBN", SqlDbType.VarChar).Value = items.Cells["ISBN"].Value.ToString();
                            cmd.Parameters.Add("@Name", SqlDbType.VarChar).Value = items.Cells["Name"].Value.ToString();
                            cmd.Parameters.Add("@Description", SqlDbType.VarChar).Value = items.Cells["Description"].Value.ToString();
                            cmd.Parameters.Add("@PublishDate", SqlDbType.DateTime).Value = DateTime.Parse(items.Cells["Publish Date"].Value.ToString());
                            cmd.Parameters.Add("@Language", SqlDbType.VarChar).Value =  items.Cells["Language"].Value.ToString();
                            cmd.Parameters.Add("@AuthorID", SqlDbType.VarChar).Value = items.Cells["Author"].Value.ToString();
                            cmd.Parameters.Add("@Category", SqlDbType.VarChar).Value = items.Cells["Category"].Value.ToString();
                            cmd.Parameters.Add("@Edition", SqlDbType.Int).Value = int.Parse(items.Cells["Edition"].Value.ToString());
                            cmd.Parameters.Add("@Price", SqlDbType.Decimal).Value = decimal.Parse(items.Cells["Price"].Value.ToString());

                            conn.Open();
                            cmd.ExecuteNonQuery();

                        }
                    }

                }

                if (line > 0)
                {
                    clearRecord();
                    clearBookRecord();
                    viewBookID();
                    viewBookRecord();
                    CusNotifi.showSuccess("Data Inserted Succesfully!");
                }
                else
                {
                    CusNotifi.showError("Data Insert Failed!");
                }
            }
            else
            {
                CusMsgBox cmb = new CusMsgBox();
                cmb.Dispose();
            }
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.ToString(), "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
        }
    }

Note I've handled everything in addRecord(); I didn't think the other was needed. I believe that as I'm using Using blocks, the parameters, connections, etc, will be disposed of gracefully as well, before the next statement is executed.

If this needs a little nudge in the right direction, someone far better at C# please edit or leave a comment.

score 0 · Answer 3 · edited Oct 16 '19 at 18:49

0

Try placing "AuthorID" instead of "Author" on the line:

bookDetailInfo.AuthorID = items.Cells ["Author"]. Value.ToString ();

from Book.cs

edited Oct 16 '19 at 18:49

marc_s

732,580
175
1,330
1,459

answered Oct 16 '19 at 17:27

Dalton Garbin

70
1
1
8

I don't think this is relevant. You are comparing a stored procedure variable to the ID of a cell in the grid. As he documents, the problem is on the INSERT of the stored procedure. – JohnPete22 Oct 16 '19 at 17:29

Procedure or function SP_InsertBookDetail has too many arguments specified

3 Answers3