1

I get this error when I try to add a new person to my listbox and into my database.

My code:

public void AddSpeler(string name, string club)
        {




        conn.Open();


            Speler speler = new Speler();
            speler.Name = name;
            speler.Club = club;


            string query = "INSERT INTO Speler OUTPUT Inserted.ID VALUES ('" + speler.Name + "', '" + speler.Club + "')";
            SqlCommand cmd = new SqlCommand(query, conn);


            speler.ID = (int)cmd.ExecuteScalar();

            conn.Close();
        }

I get the error on the part:

 "speler.ID = (int)cmd.ExecuteScalar();

And yes, I have my primary key (ID) set to increase by one.

Fay007
  • 2,707
  • 3
  • 28
  • 58
Mike Hunt
  • 37
  • 1
  • 1
  • 7
  • 1
    First you should use parameters and not concatenate strings, second, if you don't supply the column names then you should supply a value for each column present in the datatable – Steve Jan 08 '17 at 13:21

2 Answers2

2

As said in the comment above, you shouldn't write an sql command concatenating strings. This is well known to be a source of bugs and a big security risk called Sql Injection.

The second point to fix is the syntax used. In an INSERT INTO statement you should provide the column that matches the values inserted. If you don't supply the column names then you need to write the values for each column in the table and in the exact order in which the columns are defined

void AddSpeler(string name, string club)
{
    conn.Open();
    Speler speler = new Speler();
    speler.Name = name;
    speler.Club = club;
    string query = @"INSERT INTO Speler (Name, Club) OUTPUT Inserted.ID 
                     VALUES (@name, @club)";
    SqlCommand cmd = new SqlCommand(query, conn);
    cmd.Parameters.Add("@name", SqlDbType.NVarChar).Value = speler.Name;
    cmd.Parameters.Add("@club", SqlDbType.NVarChar).Value = speler.Club;
    speler.ID = (int)cmd.ExecuteScalar();
    conn.Close();
}

Here I assume that your table Speler contains the columns named Name and Club, of course change them to your actual names.

EDIT
If you want to call this method from another class, you need to make it public so every callers that creates an instance of the class where the method is defined can use it.

By the way, the code in AddSpeler does some things that are wasted if you don't return them to your caller (void ??) Possibly you want to return the instance of the Speler class created in the code so change the method to

public Speler AddSpeler(string name, string club)
{
    try
    {
       conn.Open();
       Speler speler = new Speler();
       .....

       ....
       return speler;
    }
    catch(Exception ex)
    {
        // display the ex.Message to know why your code fails
        MessageBox.Show(ex.Message);
        conn.Close();
        return null; // no Speler returned if code fails
    }
}
Community
  • 1
  • 1
Steve
  • 213,761
  • 22
  • 232
  • 286
  • Do I need to change anything from 'SqlDbType.NVarChar' because now I get an error that SqlDbType does not exist in this current context. – Mike Hunt Jan 08 '17 at 13:45
  • SqlDbType is an enum defined in the namespace System.Data. Probably you need only to add the requested _using System.Data;_ at the beginning of your file – Steve Jan 08 '17 at 13:46
  • However, the NVarChar should match the definition of your column. Usually the strings are mapped to NVarChar columns but check your table definition – Steve Jan 08 '17 at 13:47
  • Addind _using System Data_ at the beginning seems to have fixed that error, but it now gives me another one ''Administration.AddSpeler(string, string)' is inaccessible due to its protection level' Administration is the class in which I have the code above. And now I can't call it in my main code? – Mike Hunt Jan 08 '17 at 13:50
  • If you are calling this method from another class you need to make it public. Add _public_ to _void AddSpeler(.....)_ – Steve Jan 08 '17 at 13:53
  • Thanks, that fixed it. Another question if you don't mind: **Cannot insert the value NULL into column 'ID', table 'Speler.mdf.dbo.Speler'; column does not allow nulls. INSERT fails.** I thought I shouldn't get this error because I had my ID to automatically increase because I set it's Indentity Specification to true. ? Any idea? – Mike Hunt Jan 08 '17 at 13:59
  • That's exactly the way to do it. Cannot answer to this. I suggest to check accurately your table definition and eventually post a new question where more people will look at the specific problem – Steve Jan 08 '17 at 14:00
0

The problem is that you want to skip the ID column in the values provided, so you need to specify that.

    string query = "INSERT Speler (Name, Club) OUTPUT Inserted.ID VALUES ('" + speler.Name + "', '" + speler.Club + "')";

While this will solve the error, you should absolutely not concatenate SQL strings! Use parameters instead.

Lucero
  • 59,176
  • 9
  • 122
  • 152