-1

So the thing is, im trying to insert 2 dates into my local database using c#, the date values are taken from 2 different datetimepickers called dtp1 and dtp2 but i dont know how to do that, i tried the following:

connect.Open();
command = new SqlCommand("insert into ab (id, name, data1, data2) values ("+int.Parse(textbox_id.Text)+", '"+textBox_name.Text+"', "+dtp1.Value+","+dtp2.Value+")",connect);
command.ExecuteNonQuery();
connect.Close();

The error is : incorrect syntax

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
christian_chr
  • 77
  • 2
  • 11
  • What is the complete error message exactly? What is your column types? And you should _always_ use parameterized statements. This kind of string concatenations are open for SQL Injection attacks. – Soner Gönül Nov 15 '15 at 18:26
  • 1
    Have you ever heard about parameterized queries? – Steve Nov 15 '15 at 18:27

2 Answers2

2

First let's fix the SQL injection vulnerability in your code. Create a query with parameters:

command = new SqlCommand("insert into ab (id, name, data1, data2) values (@id, @name, @data1, @data2)",connect);

Then you can simply add values for those parameters, without having to worry about their string representations (rendering your current problem moot):

command.Parameters.AddWithValue("@id", int.Parse(textbox_id.Text));
command.Parameters.AddWithValue("@name", textBox_name.Text);
command.Parameters.AddWithValue("@data1", dtp1.Value);
command.Parameters.AddWithValue("@data2", dtp2.Value);

The benefits here include:

  • Much cleaner and easier to read code
  • You can use values as-is rather than trying to convert them to specific string representations
  • Most importantly, you're treating user input as values rather than as executable code
David
  • 208,112
  • 36
  • 198
  • 279
1
  1. Use SqlParameter to be sure no sql injection is possible: How does SQLParameter prevent SQL Injection?.

  2. If you use string concation, try to use string.Format to get things more clear: string.Format("insert into ab (id, name, data1, data2) values ({0}, '{1}', {2}, {3})", textbox_id.Text, textBox_name.Text, dtp1.Value, dtp2.Value). But in general this is not a good idea to pass parameter.

  3. Why do you use int.parse if you make a ToString later when concation? This is not needed: ("+int.Parse(textbox_id.Text)+",. This is the same like this: ("+(int.Parse(textbox_id.Text)).ToString()+", which is the same like: textbox_id.Text. Besides it throws an exception if textbox_id.Text does not contains some thing that could be parsed into an integer. If you want to check for integer, just use int.TryParse.

  4. You may get some syntax error if dtp1.Value or dtp2.Value is empty or if you try to pass some thing besides integer.

Using SqlParamter for DateTime even has the advantagem that you don't have to care about the DateTime format. Which can be really annoying.

Community
  • 1
  • 1
BendEg
  • 20,098
  • 17
  • 57
  • 131