C# Foreach value in a using clause?

Question

I'm trying to loop through an array of strings using the foreach control, and then use each value to insert information in the database. Could someone help me understand why within the using clause I can't use the foreach variable?

string[] ship_ids = ShipsInScope.Split('|');
foreach (string ship_id in ship_ids)
{
     using (SqlCommand InsertCommand = new SqlCommand("insert into PROJECT_SHIP (CR_Number, Ship_Id) VALUES (@CR_Number, @CCF_Number)", DBConn))
    {
         InsertCommand.Parameters.Add("@CR_Number", SqlDbType.NVarChar, 10).Value = CRNumber;
         InsertCommand.Parameters.Add("@Ship_Id", SqlDbType.NVarChar, 10).Value = Ship_Id;

         InsertCommand.ExecuteNonQuery();
         InsertCommand.Dispose();
    }
}

you have a casing problem there, in C# ship_id is not the same as Ship_id — Marek, Jan 16 '12 at 13:05
You don't need a call to `InsertCommand.Dispose()` *within* your `using` clause as the `using` clause will automatically call `Dispose` on the `SqlCommand` when the code reaches the end of it. — Samuel Slade, Jan 16 '12 at 13:05
Why are you calling `InsertCommand.Dispose();` within the `using` block? — Oded, Jan 16 '12 at 13:06
What they said. Also, I would never create a SqlCommand inside a loop like that. Create the SqlCommand once, then use it in the loop. Edit: oh well, it's not like you open and close the SqlConnection every time. — Mr Lister, Jan 16 '12 at 13:07

Jon Skeet · Accepted Answer · 2012-01-16T13:12:06.697

C# is case-sensitive. Your iteration variable is ship_id but you're trying to use Ship_Id in the loop.

Ideally, use C# naming conventions instead (and for other variables too):

// Declared outside the method.
private const string InsertSql = 
    "insert into PROJECT_SHIP (CR_Number, Ship_Id) " +
    "VALUES (@CR_Number, @CCF_Number)";

...

string[] shipIds = ShipsInScope.Split('|');
foreach (string shipId in shipIds)
{
    using (SqlCommand command = new SqlCommand(InsertSql, connection))
    {
        command.Parameters.Add("@CR_Number", SqlDbType.NVarChar, 10)
                          .Value = crNumber; // Unclear what this means
        command.Parameters.Add("@Ship_Id", SqlDbType.NVarChar, 10)
                          .Value = shipId;
        command.ExecuteNonQuery();
    }
}

Notes:

Extracted constant SQL into a class-level constant. Not necessary, but might clarify things.
Renamed all variables to be camelCase without underscores
Wrapped lines for StackOverflow - you probably don't need as much wrapping in your code
Removed redundant explicit call to Dispose (as the using statement already calls Dispose)

Unbelievable guys, that was an incredibly fast response time. Apologies it was so simple, I'm doing this after coming from VB! Thanks for the tip RE the using block. — deed02392, Jan 16 '12 at 13:10

score 3 · Answer 2 · answered Jan 16 '12 at 13:06

You are using Ship_id instead of ship_id. C# is case sensitive.

string[] ship_ids = ShipsInScope.Split('|');
foreach (string ship_id in ship_ids)
{
     using (SqlCommand InsertCommand = new SqlCommand("insert into PROJECT_SHIP (CR_Number, Ship_Id) VALUES (@CR_Number, @CCF_Number)", DBConn))
    {
         InsertCommand.Parameters.Add("@CR_Number", SqlDbType.NVarChar, 10).Value = CRNumber;
         InsertCommand.Parameters.Add("@Ship_Id", SqlDbType.NVarChar, 10).Value = ship_Id;

         InsertCommand.ExecuteNonQuery();
    }
}

Additionally, the using block will end up calling Dispose on InsertCommand - that's what a using statement does. No need to call Dispose yourself.

score 2 · Answer 3 · answered Jan 16 '12 at 13:09

2

if you are using the using statement you don't need to call the Dispose() method, it calls internally, and the names of the parameters are different, in the sql statement is @CCF_Number and in the parameters section is @Ship_Id.

answered Jan 16 '12 at 13:09

Gustavo F

2,071
13
23

score 1 · Answer 4 · answered Jan 16 '12 at 13:07

1

You capitalized your "ship_id" variable name:

             InsertCommand.Parameters.Add("@Ship_Id", SqlDbType.NVarChar, 10).Value = Ship_Id

answered Jan 16 '12 at 13:07

doogle

3,376
18
23

Ɖiamond ǤeezeƦ · Answer 5 · 2012-01-16T13:55:11.860

1

Based on the answers and comments so far, you could restructure your code as follows:

const string sql = @"
    INSERT INTO PROJECT_SHIP (CR_Number, Ship_Id) 
    VALUES (@CR_Number, @Ship_Id)";

using (SqlCommand InsertCommand = new SqlCommand(sql, DBConn))
{
    var parameters = InsertCommand.Parameters;
    var crNumberParameter = parameters.Add("@CR_Number", SqlDbType.NVarChar, 10); 
    var shipIdParameter = parameters.Add("@Ship_Id", SqlDbType.NVarChar, 10);
    string[] ship_ids = ShipsInScope.Split('|');
    foreach (string ship_id in ship_ids)
    {
        crNumberParameter.Value = CRNumber;
        shipIdParameter.Value = ship_id;
        InsertCommand.ExecuteNonQuery();
    }
}

edited Jan 16 '12 at 13:55

answered Jan 16 '12 at 13:25

Ɖiamond ǤeezeƦ

3,223
3
28
40

Have you tried compiling this? I'm spotting a variable named like a C# keyword... – Nuffin Jan 16 '12 at 13:34
@Tobias - at least some one reads my answers! Thanks for the feedback. That will teach me to make edits to my code on the fly. I added the `params` variable to stop the horizontal scroll bar appearing (I find horizontal scrolling makes answers less readable - not sure if any one else does though!). Renamed variable to `parameters` instead. – Ɖiamond ǤeezeƦ Jan 16 '12 at 13:57

score 0 · Answer 6 · answered Jan 16 '12 at 13:18

0

On another note,

instead of sending 10000 of Insert commands to SQLServer in so many operations

when you can create a script that will Add the entire collection in a single operation.

think it is more effective.

answered Jan 16 '12 at 13:18

Tomer W

3,395
2
29
44

Ordinarily that's what I'd do but I come from a PHP+MySQL background and am not sure about collecting multiple inserts and doing them at once in C#... – deed02392 Jan 16 '12 at 16:34

C# Foreach value in a using clause?

6 Answers6