0

I have two radio boxes. One true, the other false. I have a flag set up, but it didn't work. Here is my code right now

string sqlStatement;
SqlConnection cnn = new SqlConnection(Properties.Settings.Default.cnnString);

cnn.Open();
SqlCommand cmd = cnn.CreateCommand(); 
cmd.CommandType = CommandType.Text;
sqlStatement = string.Format("UPDATE Results SET Finish = '{0}', Place = '{1}', FinishTime = {2}, Winnings = '{3}' Where ResultsId = '{4}' " + (rdoDidFinish.Checked ? 1 : 0), txtPlace.Text, txtTime.Text, txtWinnings.Text);

cmd.CommandText = sqlStatement;
cmd.ExecuteNonQuery();
cnn.Close();

The error I am getting is:

"Index (zero based) must be greater than or equal to zero and less than the size of the argument list."

7
  • 2
    Please use parameters in a prepared statement!
    – juergen d
    Commented Mar 29, 2018 at 13:49
  • What do you mean @juergend
    – Brad
    Commented Mar 29, 2018 at 13:50
  • Scroll to C# in this page:rosettacode.org/wiki/Parametrized_SQL_statement
    – juergen d
    Commented Mar 29, 2018 at 13:51
  • 3
    You have 5 placeholders in your String.Format() call but are only passing 3 arguments. Also, as pointed out, you should really be using parameters rather than building your SQL statement like that as you're leaving yourself open to SQL Injection attacks.
    – Diado
    Commented Mar 29, 2018 at 13:51
  • This isn't a SQL error, this is a string.format error. You have five parameters, but only provide 3 in your statement. Commented Mar 29, 2018 at 13:51

3 Answers 3

2

I see a few problems here.

NEVER EVER build a SQL Statement by piecing together a string and values, doesn't matter if it is stringbuilder or string.format, or what not. This is language agnostic; DON'T do it in any language. For more information on the dangers, please Google "SQL Injection" The best thing to do is use paramaterized queries. Sample will follow.

Your problem is actually in your string.Format line. There are 2 errors here

Error 1: I believe the + should be a , here: '{4}' " + (rdoDidFinish.Checked ? 1 : 0)

Error 2 You have 5 placeholders, and only 3 values. 4 if the above (Err 1) statement is corrected

Here is a simple mock up assuming Err 1 fixed.. Need to know what the ResultID value is.

string sqlStatement = "UPDATE Results SET Finish = @Finish, Place = @Place, FinishTime = @FinishTime, Winnings = @Winnings WHERE ResultsId = @ResultID";

using (SqlConnection cnn = new SqlConnection(Properties.Settings.Default.cnnString)) {
    SqlCommand cmd = cnn.CreateCommand(sqlStatement, cnn);
    cmd.CommandType = CommandType.Text;

    cmd.Parameters.AddWithValue("@Finish", (rdoDidFinish.Checked ? 1 : 0));     // I think this belongs here.
    cmd.Parameters.AddWithValue("@Place", txtPlace.Text);
    cmd.Parameters.AddWithValue("@FinishTime", txtTime.Text);
    cmd.Parameters.AddWithValue("@Winnings", txtWinnings.Text);
    // cmd.Parameters.AddWithValue("@ResultID", );                                ? What belongs here ?

    cnn.Open();
    cmd.ExecuteNonQuery();
    cnn.Close();

}

0

Please use parameters in a prepared statement!

Example:

var myParam = new SqlParameter();
myParam.ParameterName = "@myParam";
myParam.Value = 1;
comm.Parameters.Add(myParam);

You can use the parameter in your SQL-Statement like

Select * from table where abc = @myparam
1
  • 2
    This should be a comment - it does not answer the question of OP. Commented Mar 29, 2018 at 14:12
0

Your Where ResultsId = '{4}' does not currently have any values being inserted into it.

Add another parameter after txtWinnings.txt providing the ResultsId and replace the + with a , before (rdoDidFinish.Checked ? 1 : 0) so that it is recognized as its own parameter.

The correct statement should look something like this, where resultId is whatever id you want to search for:

sqlStatement = string.Format("UPDATE Results SET Finish = '{0}', Place = '{1}', FinishTime = {2}, Winnings = '{3}' Where ResultsId = '{4}' ", (rdoDidFinish.Checked ? 1 : 0), txtPlace.Text, txtTime.Text, txtWinnings.Text, resultId);

One final note, in line with the comments on your question, you should always prepare your parameters. So your final result should be something along the lines of:

cmd.CommandText = @"UPDATE Results SET Finish = @finished, Place = @place, FinishTime = @time, Winnings = @winnings Where ResultsId = @resultId ";
cmd.Parameters.Add(new SqlParameter("@finished", System.Data.SqlDbType.Bit).Value = rdoDidFinish.Checked);
cmd.Parameters.Add(new SqlParameter("@place", System.Data.SqlDbType.VarChar).Value = txtPlace.Text);
cmd.Parameters.Add(new SqlParameter("@time", System.Data.SqlDbType.VarChar).Value = txtTime.Text);
cmd.Parameters.Add(new SqlParameter("@winnings", System.Data.SqlDbType.VarChar).Value = txtWinnings.Text);
cmd.Parameters.Add(new SqlParameter("@resultId", System.Data.SqlDbType.Int).Value = resultId);
2
  • Look closely; the rdoDidFinish value would actually be concatenated to the string. It is not seperated via a comma
    – Mad Myche
    Commented Mar 29, 2018 at 14:17
  • Thanks, I was just editing my answer to fix that when I noticed it. :)
    – Chris
    Commented Mar 29, 2018 at 14:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.