0

I'm trying to write a CRUD app and I have a problem with the Create method. Program is crashing with error

System.Data.SqlClient.SqlException: 'Violation of PRIMARY KEY constraint 'PK_Pracownicy'. Cannot insert duplicate key in object 'dbo.Pracownicy'. The duplicate key value is (11). The statement has been terminated.'

But I can see in my SQL Server that this position is added to Pracownicy table, so I don't know where is a problem. Its look like the error is generated after I put new values to table

enter image description here

class SqlHelper
{
    public int RowsLenght { get; set; }
    private SqlConnection sqlConnection;
    public string Command { get; set; }

    public SqlHelper(string connectionString)
    {
        sqlConnection = new SqlConnection(connectionString);
        sqlConnection.Open();
       
    }
    public int Create(string command, SqlParameter []parameters)
    {
        //wykonanie polecenia na bazie
        using var cmd = new SqlCommand(command, sqlConnection);
        cmd.Parameters.AddRange(parameters);
        ShowTable(cmd);
        return cmd.ExecuteNonQuery();
    }
    public int Read(string command)
    {
        //wykonanie polecenia na bazie
        using var cmd = new SqlCommand(command, sqlConnection);
        ShowTable(cmd);
        return cmd.ExecuteNonQuery();
    }
    private int ShowTable(SqlCommand command)
    {
        var reader = command.ExecuteReader();
        while (reader.Read())
        {
            Console.WriteLine(reader.GetInt32(0) + "\t" + reader.GetString(2) + " " +
                reader.GetString(1) + "\t" + reader.GetString(3));
            RowsLenght++;
        }
        reader.Close();
        return RowsLenght;
    }
}

class Program
{
    static void Main()
    {
        SqlConnectionStringBuilder connectionString = new SqlConnectionStringBuilder
        {
            DataSource = @"HAL\MSSERVER",
            InitialCatalog = "ZNorthwind",
            IntegratedSecurity = true,
            ConnectTimeout = 30,
            Encrypt = false,
            TrustServerCertificate = false,
            ApplicationIntent = 0,
            MultiSubnetFailover = false
        };
        var connectionS = connectionString.ConnectionString;
        SqlHelper sql = new SqlHelper(connectionS);

        var command = "SELECT * FROM dbo.Pracownicy";
        sql.Read(command);

        command = "INSERT INTO dbo.Pracownicy (IDpracownika, Nazwisko, Imię, Stanowisko) VALUES (@IDpracownika, @Nazwisko, @Imie, @Stanowisko)";
        var parameters = SetUpParameters(sql.RowsLenght);
        sql.Create(command, parameters);
    }
    private static SqlParameter[] SetUpParameters(int lenght)
    {
        //inicjalizacja zmiennych:
        Console.WriteLine("Podaj imie nowego pracownika: ");
        var fname = Console.ReadLine();
        Console.WriteLine("Podaj nazwisko pracownika: ");
        var lname = Console.ReadLine();
        Console.WriteLine("Podaj stanowisko pracownika: ");
        var position = Console.ReadLine();

        SqlParameter []param = new SqlParameter[4];
        param[0] = new SqlParameter("IDpracownika", lenght + 1);
        param[1] = new SqlParameter("Imie", fname);
        param[2] = new SqlParameter("Nazwisko", lname);
        param[3] = new SqlParameter("Stanowisko", position);

        return param;
    }
}

Thanks

Dale K
  • 25,246
  • 15
  • 42
  • 71
mikeyMike
  • 21
  • 4
  • 1
    cmd.ExecuteNonQuery() returns an integer indicating the number of rows changed. You are using an INSERT command with a table that has a primary key. If the key already exists you cannot add the key instead you need to update. So if cmd.ExecuteNonQuery() returns zero than you need to repeat operation with an update command. – jdweng Mar 16 '21 at 18:36
  • 1
    Well these errors are common when you try to write a do_it_all database class. Best to invest a bit of your time in learning an ORM library like Dapper. Now, your way to calculate the next value for your primary key is less than optimal. I would say plainly wrong. What do you think will happen if you delete the row with pk = 1 and then count the rows to find the next pk value? Learn how to use IDENTITY values as pk pf your table – Steve Mar 16 '21 at 18:40
  • well, this is for my profesor, i wrote other program in Dapper and it works just fine, but i have to correct this one. And its doesn't matter what number i put as pkey, i got the same error – mikeyMike Mar 16 '21 at 18:45
  • 1
    This class *guarantees* PK violations and corruption. On one hand, it can only work if only a single connection is modifying the data. If *any* other connection adds or deletes rows, `RowsLength` will be wrong. On the other hand, a simple deletion would mean the next attempt to calculate `RowsLength` would return an *old* ID value. If other tables contain the old ID value, they'll end pointing to the wrong record – Panagiotis Kanavos Mar 16 '21 at 18:48
  • @mikeyMike `this is for my profesor` he won't be pleased at all – Panagiotis Kanavos Mar 16 '21 at 18:48
  • A workaround would be using the _SELECT MAX(IDpracownika) from ...._ and then increment the result by one. But you will be in the same water if your program is run concurrently as explained above. As I have told you, the IDpracownika field should be marked as IDENTITY and let the database calculate the next value for your records without using it or passing a value for it in the insert query. – Steve Mar 16 '21 at 18:55
  • If you have been told to create your own ID, use a GUID. Do not try to implement an autoincrement feature. Otherwise, use the database's Identity field capabilities. – insane_developer Mar 16 '21 at 19:12
  • Effectively you are using [addwithvalue](http://www.dbdelta.com/addwithvalue-is-evil/) when creating your parameters which is bad. And that is something your teacher should teach - best practices. – SMor Mar 16 '21 at 19:33
  • I can't get my head round what that `SELECT *` is supposed to do. You should also be disposing your connection object with `using`, **do not cache it** – Charlieface Mar 16 '21 at 19:46

0 Answers0