One part of my c# coding of adding items to a database is not working. What can be the reason?

Question

There are text boxes in my windows form called txtName,txtAdd,txtTel & txtEmail. I need to add the text in this text boxes to my database table. My table name is "Table". I used the following code to add those text to my table. The table columns are Name,Address,Tel No. & email.

 SqlConnection con = new SqlConnection(@"Data Source=(LocalDB)\v11.0;AttachDbFilename=c:\users\user\documents\visual studio 2012\Projects\WindowsFormsApplication1\WindowsFormsApplication1\Database1.mdf;Integrated Security=True");

    private void button1_Click(object sender, EventArgs e)
    {
        try
        {
            String name = txtName.Text;
            String add = txtAdd.Text;
            String tel = txtTel.Text;
            String email = txtEmail.Text;

            String SqlQuery = "insert into Table values('" + name + "','" + add + "','" + tel + "','" + email + "')";
            SqlCommand cmnd = new SqlCommand(SqlQuery, con);
            con.Open();
            cmnd.ExecuteNonQuery();
            MessageBox.Show("Saved Successfully", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information);
        }
        catch
        {
            MessageBox.Show("Error occured while saving", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information);
        }
        finally
        {
            con.Close();
        }
    }

I typed some texts and pressed button1. But everytime I get the "Error occured while saving" message box. I can't understand the reason for that. I used varchar data type for all of the fields in my table. I am using visual studio 2012 express. Please help me to solve this problem.

Answer 1

The problem is caused by your string concatenation of values to form an sql command. This is well known as a source of bugs and in a potential security risk called Sql Injection. Otherwise you could receive an error when using an INSERT INTO table without specifying the column names. If you don't write the columns then you should pass a value for every column of your table and in the exact order expected by your table.

You should use a parameterized query

public string conString = @"Data Source=(LocalDB)\v11.0;
 AttachDbFilename=c:\users\user\documents\visual studio 2012\Projects\WindowsFormsApplication1\WindowsFormsApplication1\Database1.mdf;
 Integrated Security=True";


private void button1_Click(object sender, EventArgs e)
{
    try
    {
        String name = txtName.Text;
        String add = txtAdd.Text;
        String tel = txtTel.Text;
        String email = txtEmail.Text;
        String SqlQuery = @"insert into [Table] 
                            values(@name,@add,@tel,@mail)";

        using(SqlConnection con = new SqlConnection(conString))
        using(SqlCommand cmnd = new SqlCommand(SqlQuery, con))
        {
             con.Open();
             cmnd.Parameters.Add("@name", SqlDbType.NVarChar).Value = name;
             cmnd.Parameters.Add("@add", SqlDbType.NVarChar).Value = add;
             cmnd.Parameters.Add("@tel", SqlDbType.NVarChar).Value = tel;
             cmnd.Parameters.Add("@email", SqlDbType.NVarChar).Value = email;
             cmnd.ExecuteNonQuery();
         }
     }
     catch(Exception ex)
     {
          MessageBox.Show(ex.Message, "Message", MessageBoxButtons.OK, MessageBoxIcon.Information);
     }
}

Notice also that I have moved your disposable objects inside a using block to ensure proper disposing of the objects SqlConnection and SqlCommand

EDIT If you really have a table in your database named Table then you need to put square brackets around that name because it is a reserved word in Sql Server (or better, change that name to something more meaningful in relation to the data stored there)