foreach loop does loop, then get error Procedure or function spTestSp has too many arguments specified

Question

I cannot work out what I'm doing wrong on this foreach loop, it insert 1st set of records, then return error; Procedure or function spTestSp has too many arguments specified.

Can anyone point me in the right direction

List<Jobs> jobDetails = new List<Jobs>
{
    new Jobs {JobDate = "Job3",JobRrf = "ref3"},
    new Jobs {JobDate = "Job4",JobRrf = "REf4"},
    new Jobs {JobDate = "Job5",JobRrf = "ref5"},
    new Jobs {JobDate = "Job6",JobRrf = "REf6"},
};

const string spName = "dbo.spTestSp";

using (var cn = new SqlConnection(_dbConn))
{
    using (var cmd = new SqlCommand(spName, cn))
    {
        //cn.Open();
        foreach (var data in jobDetails)
        {

            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Parameters.AddWithValue("@param1", data.JobDate);
            cmd.Parameters.AddWithValue("@param2", data.JobRrf);
            //cmd.Parameters.AddWithValue("@param2", data.JobRrf);
            cn.Open();
            cmd.ExecuteNonQuery();
            cn.Close();
        }
        //cn.Close();
    }
}

alter PROCEDURE spTestSp
    -- Add the parameters for the stored procedure here
    @param1 nvarchar(50),
    @param2 nvarchar(50)
AS
BEGIN
    -- SET NOCOUNT ON added to prevent extra result sets from
    -- interfering with SELECT statements.
    SET NOCOUNT ON;

    -- Insert statements for procedure here
    insert into [dbo].[tblTest]([test1],[test2])values(@param1,@param2)

END

You'll need to re-create the `SqlCommand` instance _inside_ the `foreach` loop. — Anton Gogolev, Jan 27 '15 at 14:59
Or just Clear the parameters or set them outside the loop and reset the values. — Crowcoder, Jan 27 '15 at 15:03
Also, [beware](http://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/) of using [AddWithValue](http://stackoverflow.com/questions/9155004/difference-with-parameters-add-and-parameters-addwithvalue), though it shouldn't matter in this case. — James Thorpe, Jan 27 '15 at 15:05
Normally when I am working with the same SP I will add the paramaters to the query out side the loop and then just assign them inside. One of the coding patterns that I have picked up is to try and avoid `new` in a loop if you can. Probably a hold over from some old language I dont know where memory management was a pain but it still makes code easier to read IMHO. — RubberChickenLeader, Jan 27 '15 at 15:08

score 3 · Accepted Answer · answered Jan 27 '15 at 15:11

Personally I can't see any advantages of creating only one SqlCommand object, and reuse it in foreach block. As you can clear it's parameters, it's easier to recreate the whole command, like this:

cn.Open();
foreach (var data in jobDetails)
{
    using (var cmd = new SqlCommand(spName, cn))
    {
        cmd.CommandType = CommandType.StoredProcedure;
        cmd.Parameters.AddWithValue("@param1", data.JobDate);
        cmd.Parameters.AddWithValue("@param2", data.JobRrf);
        //cmd.Parameters.AddWithValue("@param2", data.JobRrf);
        cmd.ExecuteNonQuery();
    }
}
//cn.Close();

In your code you are signalling the server to close connection and move it to the connection pool after each command, which isn't very efficient. Open it once and send your data to the server. Also make note that using block for the SqlConnection object will close it automatically, so you don't have to make it manually.

As for your error, it occurs simply because of the too many parameters you have in your command after one loop cycle.

foreach loop does loop, then get error Procedure or function spTestSp has too many arguments specified

1 Answers1