0

I am trying to insert a new row into a SQL Server table from a Winforms application. As far as I know my query is correct but Visual Studio keeps returning an error:

Incorrect syntax near 'achternaam'

I hope that someone can point me in the right direction.

public void UpdateGegevens(int id, string voornaam, string achternaam, string functie, DateTime geboortedatum, decimal uurloon)
{
if (ReturnFirstTime(id) == true)
{
    using (SqlConnection con = new SqlConnection(connectionString))
    {
        using (SqlCommand command = new SqlCommand())
        {
            command.Connection = con;
            command.CommandType = CommandType.Text;
            command.CommandText = "INSERT INTO tbl_Gegevens (Id, voornaam, achternaam, geboortedatum, functie, uurloon) VALUES (@Id, @vn, @an, @gb, @f, @ul);";

            command.Parameters.Add("@Id", SqlDbType.Int).Value = id;
            command.Parameters.Add("@vn", SqlDbType.VarChar).Value = voornaam;
            command.Parameters.Add("@an", SqlDbType.VarChar).Value = achternaam;
            command.Parameters.Add("@f", SqlDbType.VarChar).Value = functie;
            command.Parameters.Add("@gb", SqlDbType.Date).Value = geboortedatum;
            command.Parameters.Add("@ul", SqlDbType.Money).Value = uurloon;

            try
            {
                con.Open();
                command.ExecuteScalar();
            }
            catch (SqlException ex)
            {
                System.Windows.Forms.MessageBox.Show(ex.Message);
            }
            finally
            {
                con.Close();
            }
        }
    }
}
        else
        {
            using (SqlConnection con = new SqlConnection(connectionString))
            {
                using (SqlCommand command = new SqlCommand())
                {
                    command.Connection = con;
                    command.CommandType = CommandType.Text;
                    command.CommandText = "UPDATE tbl_Gegevens SET voornaam=@vn achternaam=@an geboortedatum=@gb funtie=@f uurloon=@ul WHERE Id = @Id;";
                    command.Parameters.AddWithValue("@Id", id);
                    command.Parameters.AddWithValue("@vn", voornaam);
                    command.Parameters.AddWithValue("@an", achternaam);
                    command.Parameters.AddWithValue("@gb", geboortedatum);
                    command.Parameters.AddWithValue("@f", functie);
                    command.Parameters.AddWithValue("@ul", uurloon);
                    try
                    {
                        con.Open();
                        command.ExecuteNonQuery();
                    }
                    catch (SqlException ex)
                    {
                        System.Windows.Forms.MessageBox.Show(ex.Message);
                    }
                    finally
                    {
                        con.Close();
                    }
                }
            }
        }
   }

Here is a specification of tbl_Gegevens:

create table [dbo].[tbl_Gegevens] (
   [Id] int not null
 , [voornaam] nvarchar(50) null
 , [achternaam] nvarchar(50) null
 , [geboortedatum] date null
 , [functie] nvarchar(50) null
 , [uurloon] smallmoney null
 , primary key clustered ([Id] asc)
 );

I think my dbms is ADO.Net.

This is the way i'm passing the info to the method:

private void btnConfirm_Click(object sender, EventArgs e)
    {
        if (tbName.Text != "" && tbSurname.Text != "" && tbFunction.Text
            != "" && dtpBirthdate.Value != date && nudSalary.Value != 0)
            {
                Database1.SetFirstTime(ID);
                Database1.UpdateGegevens(ID, tbName.Text, tbSurname.Text, tbFunction.Text, dtpBirthdate.Value, nudSalary.Value);
                this.Hide();
                frmMain fm = new frmMain(ID);
                fm.Show();
            }
            else
            {
                MessageBox.Show("Vul alle velden in!");
            }
        }

This is the query i use to get my id:

    public int ReturnLoginID(string username, string password)
    {
        SqlConnection con = new SqlConnection(connectionString);
        SqlCommand cmd = new SqlCommand("Select * from tbl_Login where UserName=@username and Password=@password", con);
        cmd.Parameters.AddWithValue("@username", username);
        cmd.Parameters.AddWithValue("@password", password);
        int ID = 9999;
        con.Open();
        using (SqlDataReader reader = cmd.ExecuteReader())
        {
            reader.Read();
            ID = reader.GetInt32(0);
        }
        con.Close();
        return ID;
    }
  • 1
    Which DBMS? Also need table definition of `tbl_Gegevens`... – ForguesR Dec 15 '16 at 21:19
  • Here is a specification of tbl_Gegevens: CREATE TABLE [dbo].[tbl_Gegevens] ( [Id] INT NOT NULL, [voornaam] NVARCHAR (50) NULL, [achternaam] NVARCHAR (50) NULL, [geboortedatum] DATE NULL, [functie] NVARCHAR (50) NULL, [uurloon] SMALLMONEY NULL, PRIMARY KEY CLUSTERED ([Id] ASC) ); I think my dbms is ADO.Net. – Bas Molenaar Dec 15 '16 at 21:24
  • 2
    I would have used `ExecuteNonQuery` for this db action. And I would have put square brackets around `Id`. And are you sure the `Id` is writable (i.e. not an identity specification)? – Mr Lister Dec 15 '16 at 21:24
  • 1
    I don't believe this code throws that exception. Please read [ask] and provide a [mcve], – CodeCaster Dec 15 '16 at 21:32
  • I am going to try ExecuteNonQuery right now. Amazing that i didn't notice this as i know that i should only use ExecuteScalar for querys with multiple values returned. – Bas Molenaar Dec 15 '16 at 21:32
  • Is ExecuteNonQuery make any difference ? Can you execute the query in managment studio and check it ? – mybirthname Dec 15 '16 at 21:36
  • @BasMolenaar - see my comment (2nd from top). Use Sql Server Management Studio OR use Sql Server Explorer in Visual Studio. Just be sure your database is the selected active database (using a drop down or type `use databasenamehere` in SSMS) – Igor Dec 15 '16 at 21:43
  • 1
    @CodeCaster - your right. I can only reproduce that exact error message if I omit the `,` between `voornaam` and `achternaam`. Seems the `INSERT` is probably omitting the `,` in the code and the OP omitted it from the question OR did not properly execute a rebuild when testing and is using an older compiled version of the app. – Igor Dec 15 '16 at 21:49
  • 1
    `I think my dbms is ADO.Net.` <= ado.net is a library used to communicate with database management systems (dbms). An example of a dbms is Sql Server (which is probably what you are using based on the types you are using). – Igor Dec 15 '16 at 21:51
  • 1
    @BasMolenaar I see this line _if (ReturnFirstTime(id) == true)_ Did you omit the UPDATE part from the code above and, by chance, the error comes from that part? – Steve Dec 15 '16 at 21:57
  • 1
    @Bas again, if this is your actual code, this is **not the code throwing that exception**. Probably there's an error in the SELECT query in frmMain. Or what Steve says. – CodeCaster Dec 15 '16 at 21:57
  • @Bas Molenaar, maybe you should try something new: if you are using SQL Server Management Studio (SSMS), you could try writing a stored procedure with parameters. Then you can [call that stored procedure from code](http://stackoverflow.com/a/7542564/6936343) , as well as execute it by passing in the parameters using SSMS. It is better and cleaner to work that way! – Tony_KiloPapaMikeGolf Dec 15 '16 at 22:08
  • 1
    And now we see the error. Missing a comma between ALL the fields in the SET list _....SET voornaam=@vn, achternaam=@an, ....., ....._ – Steve Dec 15 '16 at 22:08
  • This is also why a stack trace is very important. If you can properly read an Exception then you can figure out the exact line number the exception occurred on. Also if you are debugging the code attached to Visual Studio it will even stop on the line where you call Execute... from the SqlCommand and then you can see the command text it was executing. Either way learn to use your debugger AND learn to read Exceptions (not just the Message property). – Igor Dec 15 '16 at 22:13
  • @Tony no, stored procedures for simple CRUD operations are a maintenance nightmare. Anyway that has been discussed to death already. Edit: nevermind, you had it under Steve's answer already. – CodeCaster Dec 15 '16 at 22:44

1 Answers1

4

In the UPDATE part of your code there are no commas to separate the fields in the SET list

command.CommandText = @"UPDATE tbl_Gegevens SET voornaam=@vn,
                       achternaam=@an, geboortedatum=@gb, 
                       funtie=@f, uurloon=@ul WHERE Id = @Id;";

I think that this question could be used to underline the importance of using a debugger. This problem would be solved much sooner if you had stepped through your code using the debugger.

Steve
  • 213,761
  • 22
  • 232
  • 286
  • Nice find! (*with the whole `ReturnFirstTime(id) == true` in the code*) – Igor Dec 15 '16 at 22:14
  • This kind of drama could have been prevented by [using stored procedures](http://stackoverflow.com/a/7542564/6936343)! – Tony_KiloPapaMikeGolf Dec 15 '16 at 22:16
  • 2
    @Tony_KiloPapaMikeGolf Stored Procedure are not the panacea of all evils. Using them for trivial tasks like these results in a maintenance nightmares if you are not wearing also the DBA hat. – Steve Dec 15 '16 at 22:17
  • Whenever I see code with SQL queries inline, I become wary, because either the programmer was lazy or in a hurry and did not want to refactor the code. A quickfix that will often cause misery on the long term. – Tony_KiloPapaMikeGolf Dec 15 '16 at 22:19
  • Also I found another stupidity, I set the first time to false before I write the data for the first time. So UPDATE was always used instead of INSERT on the first time. – Bas Molenaar Dec 15 '16 at 22:21
  • @steve, you could also use EF for your DAL. Inline SQL is acceptable, as long you are not creating huge stringbuilder nightmares! – Tony_KiloPapaMikeGolf Dec 15 '16 at 22:22
  • 1
    @Tony_KiloPapaMikeGolf Thanks. As i am still learning i did not know of it's existance. I wil use that from now on. Dankjewel ;) – Bas Molenaar Dec 15 '16 at 22:23
  • Generating Create, Read, Update, Delete StoredProcedures for tables is also available: [SSMS Tools Pack](http://ssmstoolspack.com/Features?f=12) Why write trivial code, while you can generate it on the fly? – Tony_KiloPapaMikeGolf Dec 15 '16 at 22:34