1

I don't know what is wrong with this code. It was working just fine before adding radio button. Also what is the use of primary key clustered in SQL Server?

private void createSave_Click(object sender, EventArgs e)
{
    SqlConnection con = new SqlConnection(@"Data Source=(LocalDB)\MSSQLLocalDB;AttachDbFilename=C:\Users\DELL\source\repos\phoneBookwin\phoneBookwin\Database1.mdf;Integrated Security=True");
    con.Open();
        
    string value;
    bool friendCheck = groupFriends.Checked;
    bool familyCheck = groupFamily.Checked;
    bool emergencyCheck = groupEmergency.Checked;
    bool collCheck = groupColl.Checked;

    if (friendCheck)
        value = groupFriends.Text;
    else if (familyCheck)
        value = groupFamily.Text;
    else if (emergencyCheck)
        value = groupEmergency.Text;
    else if (collCheck)
        value = groupColl.Text;
    else
        value = "";

    SqlCommand cmd = new SqlCommand("insert into Contacts(Name, Contacts, Email, Group) values('" + createName.Text + "', '" + createNumber.Text + "', '" + createEmail.Text + "','"+value+"')", con);
    cmd.ExecuteNonQuery();

    this.Hide();
    Form1 save = new Form1();

    save.ShowDialog();
}

Exception thrown:

Incorrect syntax near keyword 'group'

This is my database table:

CREATE TABLE [dbo].[Contacts] 
(
    [Name]     VARCHAR (50) NOT NULL,
    [Contacts] VARCHAR (50) NOT NULL,
    [Email]    VARCHAR (50) NULL,
    [Group]    VARCHAR (50) NULL,
    PRIMARY KEY CLUSTERED ([Name] ASC)
);
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
  • if you want to use a keyword like `Group` as a column name, I believe you have to wrap it in square brackets, like `[Group]`.Also, please use parameterized queries. Your code is open to SQL injection. – Rufus L Mar 20 '21 at 07:00
  • 2
    Please read http://bobby-tables.com - your code currently suffers from one of the biggest mistakes you can make when coding with databases, a mistake that could, at some point in your career, lead your company to [getting hacked](https://duckduckgo.com/?q=vtech+hack), you getting fired, or both. Read up on SQL injection, and never again write an sql where you directly concatenate user-supplied data into an SQL string; there is never a need for doing it because it's so easy to avoid. As a bonus your program won't explode every time someone enters an apostrophe to! – Caius Jard Mar 20 '21 at 07:04
  • Further issues: always dispose your connection object with `using`. [Avoid `AttachDbFileName`](https://stackoverflow.com/questions/11178720/whats-the-issue-with-attachdbfilename). Don't hard code connection string, store in settings config file instead. Consider how long someone's name or email can be, it may be more than 50 characters – Charlieface Mar 20 '21 at 20:50
  • Or, use Entity Framework and nearly every problem in your code right here will go away, and you won't waste your life writing boring/repetitive SQL that pushes strings around one at a time, and your program will be all round better for a host of reasons, as will your résumé – Caius Jard Mar 21 '21 at 05:58

1 Answers1

4

Your code should look like:

using(var con = new SqlConnection(@"...")
using(var cmd = new SqlCommand("insert into Contacts(Name, Contacts, Email, [Group]) VALUES(@na, @nu, @em, @gr)"), con){
    cmd.Parameters.Add("@na", SqlDbType.VarChar, 50).Value = createName.Text;
    cmd.Parameters.Add("@nu", SqlDbType.VarChar, 50).Value = createNumber.Text;
    cmd.Parameters.Add("@em", SqlDbType.VarChar, 50).Value = createEmail.Text;
    cmd.Parameters.Add("@gr", SqlDbType.VarChar, 50).Value = value;
    con.Open();
    cmd.ExecuteNonQuery();
}

Not only does it now not look like an unreadable mess of string concatenation, it's safe to use in a production environment where there are people called O'Connor or ','','',''); UPDATE Users SET Role='Admin' WHERE Name = 'Caius'--

Caius Jard
  • 72,509
  • 5
  • 49
  • 80