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();
    }
}

Answer 1

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)

Answer 2

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.

Answer 3

You capitalized your "ship_id" variable name:

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

Answer 4

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.

Answer 5

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.

Answer 6

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();
    }
}