6

I have a time sheet app where users enter their time in/out for different days of the week. The form processes the in/out from each day, stuff them as parameters into a stored procedure and add them to the database. How would I accomplish this most efficiently? I don't have access to the DB, just the stored procedures.

This is the bare code behind, I've stripped out some unnecessary codes.

SqlConnection conn = new SqlConnection(connString);
conn.Open();
SqlCommand cmd = new SqlCommand("insertINOUT", conn);
cmd.CommandType = CommandType.StoredProcedure;

cmd.Parameters.Add(new SqlParameter("@UserName", user));

for (int j = 0; j < weekDays.Length; j++)
{
    cmd.Parameters.Add(new SqlParameter("@In", in));
    cmd.Parameters.Add(new SqlParameter("@Out", out));
    cmd.ExecuteReader();
}
conn.Close();

The code works if there's only 1 day of in/out. If the users fill out multiple days, I'll get this error: Parameter '@In' was supplied multiple times.

Thanks for your help.

dam
  • 171
  • 1
  • 6
  • 17
  • It's all because of those `new` statements in the `for` loop. You're trying to make a `new` of something that already exists. – Brad Sep 13 '11 at 00:13

6 Answers6

17
SqlConnection conn = new SqlConnection(connString);
conn.Open();
SqlCommand cmd = new SqlCommand("insertINOUT", conn);
cmd.CommandType = CommandType.StoredProcedure;

for (int j = 0; j < weekDays.Length; j++)
{
    **cmd.Parameters.Clear();**
    cmd.Parameters.Add(new SqlParameter("@UserName", user));
    cmd.Parameters.Add(new SqlParameter("@In", in));
    cmd.Parameters.Add(new SqlParameter("@Out", out));
    cmd.ExecuteReader();
}
conn.Close();

(You have to clear the parameters each iteration.)

Kirk Woll
  • 76,112
  • 22
  • 180
  • 195
Kyle W
  • 3,702
  • 20
  • 32
1

It is because you were trying to re-add the same parameters to the same sqlcommand object. For best performance, BEFORE you start your for loop, open your connection and add the parameters without values. Then, INSIDE your for loop all you are doing is setting the value of the parameters and then executing the procedure. No need to recreate the parameters themselves each iteration of the loop, you're just burning resources for nothing. Try this:

string strCon = "Your Connection String Here";
using (SqlConnection conSQL = new SqlConnection(strCon))
{
    conSQL.Open();
    using (SqlCommand cmdSQL = new SqlCommand())
    {
        cmdSQL.CommandType = CommandType.StoredProcedure;
        cmdSQL.CommandText = "The Name of Your Stored Procedure Here";
        cmdSQL.Connection = conSQL;
        // I'm just going to assume that the data type for the
        // parameters is nvarchar and that both are input parameters...
        // Just for demonstration purposes
        cmdSQL.Parameters.Add("@In", SqlDbType.NVarChar, 50);
        cmdSQL.Parameters.Add("@Out", SqlDbType.NVarChar, 50);
        for (var j = 0; j <= weekDays.Length - 1; j += 1)
        {
            cmdSQL.Parameters("@In").Value = strIn;
            cmdSQL.Parameters("@Out").Value = strOut;
            // I'm not sure why in your code you put ExecuteReader here.
            // You don't show that you're using the reader at all, rather
            // it looks like you are actually just trying to execute the procedure without
            // using any type of return parameter values or a reader.
            // So I changed the code here to be what it should be if that is true.
            cmdSQL.ExecuteNonQuery();
        }
    }
    conSQL.Close();
}

I know it's been some years since this was asked, but I figured there will still be people searching on this and maybe will find this answer helpful.

1

Another alternative, you could change the scope of the SqlCommand so that it is recreated each time.

SqlConnection conn = new SqlConnection(connString);
conn.Open();

for (int j = 0; j < weekDays.Length; j++)
{
    SqlCommand cmd = new SqlCommand("insertINOUT", conn);
    cmd.CommandType = CommandType.StoredProcedure;

    cmd.Parameters.Add(new SqlParameter("@UserName", user));
    cmd.Parameters.Add(new SqlParameter("@In", in));
    cmd.Parameters.Add(new SqlParameter("@Out", out));
    cmd.ExecuteReader();
}
conn.Close();

Seems a bit wasteful, but there are some libraries that work this way (the Enterprise Library DAAB comes to mind).

mgnoonan
  • 7,060
  • 5
  • 24
  • 27
1
using (SqlConnection conn ... )
{
    SqlCommand cmd = ...
    ...
    // Set up the parameter list.
    //   You can use   .AddWithValue   here to add values that don't change in the loop.
    cmd.Parameters.Add("@Username", SqlDbType.VarChar);
    ...
    for (...)
    {
        // Load one set of loopy values.
        cmd.Parameters["@UserId"].Value = user;
        ...
    }
}
Josh Jay
  • 1,240
  • 2
  • 14
  • 26
HABO
  • 15,314
  • 5
  • 39
  • 57
0
SqlConnection conn = new SqlConnection(connString);
conn.Open();
SqlCommand cmd = new SqlCommand("insertINOUT", conn);

    cmd.CommandType = CommandType.StoredProcedure;
    cmd.Parameters.Add(new SqlParameter("@UserName", user));
    for (int j = 0; j < weekDays.Length; j++)
    {


        cmd.Parameters.Add(new SqlParameter("@In"+j, in));
        cmd.Parameters.Add(new SqlParameter("@Out"+j, out));
        cmd.ExecuteReader();
    }
    conn.Close();
0

The reason you are getting that error is because the for loop is re-adding the parameter multiple times:

cmd.Parameters.Add(new SqlParameter("@In", in));
cmd.Parameters.Add(new SqlParameter("@Out", out));

The proper way to do this is either clear the Parameters collection at the last line of the foor loop or simply check if the parameter already exists and set its value instead of doing Parameters.Add

Icarus
  • 63,293
  • 14
  • 100
  • 115