1

What is wrong in the following code? im storing the date n time into datetime field in sql server.

private void button1_Click(object sender, EventArgs e)
    {
        string d = DateTime.Now.ToShortDateString();
        cmd.CommandText = "insert into trans values("+label9.Text+",'d');";
        cmd.Connection = con;
        con.Open();
        int x= cmd.ExecuteNonQuery();
        MessageBox.Show("Attendance recorded succesfully");

3 Answers3

4

It is a very bad approach, because it opened for sql-injections. You better use SqlParameter.

cmd.CommandText="insert into trans values(@label, @date)";
cmd.Parameters.AddWithValue("label", int.Parse(label9.Text));
cmd.Parameters.AddWithValue("date", DateTime.Now);
cmd.Connection = con;
con.Open();
int x= cmd.ExecuteNonQuery();
dmitry.s
  • 658
  • 6
  • 18
2

There is mistyping in CommandText string. Use this instead

cmd.CommandText="insert into trans values("+label9.Text+","+DateTime.Now.ToString()+");";

EDIT:

Full edited code will be like this. Note that using statements will care for disposing your updates, but this code is still bad and a house of sql-injections. You must use parameters instead if you want safe code.

private void button1_Click(object sender, EventArgs e)
{
    using (System.Data.SqlClient.SqlConnection connection = new System.Data.SqlClient.SqlConnection("Data Source=localhost; Initial Datalog=myDatabase; Integrated Security=TRUE;")) 
  {
      using (System.Data.SqlClient.SqlCommand command = new System.Data.SqlClient.SqlCommand("insert into trans values("+label9.Text+","+DateTime.Now.ToString()+");", connection)) 
    {
        connection.Open();
        command.ExecuteNonQuery();
        connection.Close();
     }
  }
}
Chuck Norris
  • 14,368
  • 11
  • 83
  • 118
  • This is still vulnerable to **SQL injection** - not a good idea! And: since you concatenate together the SQL statement, you need to know what date format the target SQL Server is using - again: not a good idea. Would be **much better** to use a SQL parameter of type `DateTime` that would solve both problems with ease – marc_s Apr 11 '12 at 08:17
1

Apart from the fact that you are using inline SQL, which is just bad. You should be using @param1 syntax in the query and then adding parameters to it instead (thus sidestepping this issue also). Even better - use an ORM like Linq to Sql or Entity Framework (or nHibernate or whatever).

SQL Server generally wants it's times in yyyymmdd format, and also you really should be checking the label's value is indeed an integer and only running the query if it is:

int labelValue = 0;
if(int.TryParse(label9.Text, out labelValue))
{
  cmd.CommandText="insert into trans values("+ labelValue +
    ", '" + DateTime.Now.ToString("yyyyMMdd");"')"; 
  cmd.Connection = con;      
  con.Open();      
  int x= cmd.ExecuteNonQuery();      
  MessageBox.Show("Attendance recorded succesfully");
}     

I'd also say you really need to examine your usage of the connection/command - where do you Dispose? Judging by this code, I'm guessing you don't?

All in all, even with these fixes I'm not recommending you do things this way - do it the way that Harm suggests - the +5 (or more) there is deserved.

Andras Zoltan
  • 40,853
  • 11
  • 98
  • 156
  • For language- and dateformat-indepedent date string, use the short ISO-8601 format which is `YYYYMMDD` (**without** any dashes!) Your solution will e.g. **not** work with any of the European languages (British, German, French, Italian) - that date format will **fail** on such a SQL Server instance.... – marc_s Apr 11 '12 at 08:17
  • 1
    @marc_s okay cool, thanks have edited. On every SQL instance I've ever used this format works - but that's because they've always been the same locale no doubt (that's british, by the way). – Andras Zoltan Apr 11 '12 at 08:22
  • Better would be to not use a string to store dates/times in Sql! Use a datetime column and sql parameters. – Chris Dunaway Apr 11 '12 at 15:04
  • @ChrisDunaway, well, of course, ultimately this method of DB access stinks! And actually I just realised that there need to be quotes around the datetime literal – Andras Zoltan Apr 11 '12 at 15:29