0

I have a program which adds values to a database from textboxes.

I need to provide a way through which if the value added is not unique, it should return an error message. But my current code is unable to do that. Even if the value is not unique, it is saving the data in the database.

Below is my code.

try
        {
            if (runningExperimentToolStripMenuItem.Enabled == true && newExperimentToolStripMenuItem.Enabled == true)
            {
                MessageBox.Show("Select experiment type (under menu item 'File') first, Running or New.", "Experiment Type Required", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
            }
            else
            {
                if (sqlconf2.State == ConnectionState.Closed)
                sqlconf2.Open();
                //after connection is open, using following "if" code to check uniqueness of Step
                string query = "Select * from ExpData where 'Animal ID' = '" + textBox5.Text.Trim() + "' and Step = '" + comboBox2.Text.Trim() + "'";
                SqlDataAdapter sda = new SqlDataAdapter(query, sqlconf2);
                DataTable dtbl = new DataTable();
                sda.Fill(dtbl);
                if (dtbl.Rows.Count > 0)
                {
                    MessageBox.Show("This step has already been executed for the chosen animal ID. Please recheck. \n'Step' requires a numeric value from the drop down list. ", "Step Already Executed.", MessageBoxButtons.RetryCancel, MessageBoxIcon.Exclamation);
                }
                else
                {//code to add data into database
                    }
                }
                
                sqlconf2.Close();
            }
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message, "Error Message");
            MessageBox.Show("If problem persists, check trouble shooting options in user manual or on our website.", "Trouble Shooting.", MessageBoxButtons.OK, MessageBoxIcon.Information);
        }
        finally
        {
            datadisplay();
        }
DribbleNibble
  • 117
  • 5
  • Do you control the database, if so you can create a multicolumn unique constraint. And then wrap your db save method in a try block. This code will throw any exception if the unique constraint fails. You can catch SqlException and check for ErroCode. If ErroCode is 2627 then it is unique constraint and you can show the message. Other exceptions and error code can be handled as per the requirement. Check https://stackoverflow.com/questions/31515776/how-can-i-catch-uniquekey-violation-exceptions-with-ef6-and-sql-server – Ramesh Kanjinghat May 26 '21 at 18:42
  • @RameshKanjinghat I have already put it in the Try Catch block. I have updated my question to include it. Please take a look. I am new to coding, hoping for your help in identifying the problem here. Also note, when the argument I put is dtbl.rows.count == 0, it returns the required message box. Why is that? – DribbleNibble May 27 '21 at 04:14
  • My solution involves making changes to database and code too. If you want to stick with what you are doing now then I think the problem is in the query Column "Animal ID" has a space in that and the column is wrapped in single quotes. Single quotes makes it a literal string value instead of column name. Step1: Remove single quotes Step2: If there is space between Animal and ID then wrap it in square brackets. Try below query "Select * from ExpData where [Animal ID] = '" + textBox5.Text.Trim() + "' and Step = '" + comboBox2.Text.Trim() + "'"; – Ramesh Kanjinghat May 28 '21 at 15:04
  • There is more scope for improvement in the overall code and sql query too but for now let's get this one working. Let me know how it goes after changing the query. – Ramesh Kanjinghat May 28 '21 at 15:05
  • @RameshKanjinghat Hi. It worked beautifully. Thanks a lot. What other possible improvements were you mentioning? – DribbleNibble May 29 '21 at 16:56
  • Comment section doesn't have enough space to add my recommendations so, adding it as an answer. – Ramesh Kanjinghat Jun 01 '21 at 20:54

1 Answers1

0

There are couple of approaches to fine tune it

Without touching database

  1. Update the query to select just 1 column instead of *. * selects all the columns which is not necessary to check if the record exists or not.

string query = "Select [Animal ID] from ExpData where 'Animal ID' = '" + textBox5.Text.Trim() + "' and Step = '" + comboBox2.Text.Trim() + "'";

  1. You can also use Exists method which returns a boolean

string query = "Select Exists(Select [Animal ID] from ExpData where 'Animal ID' = '" + textBox5.Text.Trim() + "' and Step = '" + comboBox2.Text.Trim() + "')";

If we change the script then you can use sqlcommand.ExecuteScalar function to just get the value of the column you have included/boolean in the select statement.

With changes to database

  1. Add a unique key constraint on the table ExpData. The unique key should have both columns Animal ID and Step.
  2. Now in the code you can just have a try catch block. IF there is no duplicate record then the insertion will be success. But if the record is duplicate then there will be an exception.
  3. save to database code in try block
  4. In catch block check the exception type along with ErroCode to see if the insertion failed because of the unique key. Check this thread, How can I catch UniqueKey Violation exceptions with EF6 and SQL Server?

My recommended approach

  1. Add unique key constraint on both the columns
  2. Create a stored procedure that
    • takes the whole insert data as input
    • checks if the record exists then return -1 = if no record exists then insert the new data and return the id of the new record.
  3. In the code you can just check the return value. If the value is a non negative value then the insertion is successful other wise show the message to the user.

If you can change the database then you can reduce one database call if there is not duplicate data.

P.S. You can also use object–relational mapping frameworks like entity framework etc to make it much more easier but it is a big change.

Ramesh Kanjinghat
  • 721
  • 5
  • 6