user1298276
user1298276

Reputation:

Trying to insert date and time in sql server but getting error

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");

Upvotes: 1

Views: 4245

Answers (3)

Andras Zoltan
Andras Zoltan

Reputation: 42333

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.

Upvotes: 1

Chuck Norris
Chuck Norris

Reputation: 15190

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

Upvotes: 2

dmytro.s
dmytro.s

Reputation: 720

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

Upvotes: 4

Related Questions