Resolved SQLite Delete data don't work

[Kostakis45]

I'm trying to delete a record from database.

public void Delete_Info_From_DB()
        {
            Plasteka pa = new Plasteka();//Main Form

            var con = new SQLiteConnection(connection);
            con.Open();
            var cmd = new SQLiteCommand(con);
         
            try
            {
                cmd.CommandText = "DELETE FROM Description WHERE Mould_Code = '"+pa.Search_Box.Text+"'";//Am I missing something here? BTW Mould_Code is PK integer.
                cmd.ExecuteNonQuery();
            }
            catch
            {
                if ((cmd.CommandText = "SELECT * FROM Description WHERE Mould_Code = '" + pa.Search_Box.Text + "'") == null)
                {
                    MessageBox.Show("Επιτυχής διαγραφή.");

                    pa.mould_code_input.Text = null;
                    pa.machine_number_input.Text = null;
                    pa.machine_type_input.Text = null;
                    pa.supplier_input.Text = null;
                    pa.colour_input.Text = null;
                    pa.comboBox1.Text = null;
                    pa.comboBox2.Text = null;
                    pa.comboBox3.Text = null;
                    pa.numericUpDown1.Text = null;
                    pa.numericUpDown2.Text = null;
                    pa.numericUpDown3.Text = null;
                    pa.dateTimePicker1.Text = null;
                    pa.item_name_input.Text = null;
                    pa.pictureBox1.Image = null;
                    pa.pictureBox2.Image = null;
                }
                else
                {
                    MessageBox.Show("Αποτυχία διαγραφής.");
                }
            }    
            con.Close();
        }

My command line seems fine and I'm guessing that the problem lies in the reference I'm doing to get the value from textbox.But I can't figure out why it's not showing any errors,instead the program freezes.
Any idea or suggestion?

 

[cbreemer]

It looks like it ought to work - assuming the value from the textbox is ok. If you print the value of cmd.CommandText to the console and then paste it into the SQLite command interpreter, does that work and successfully delete the row(s) ?

 

[Kostakis45]

It looks like it ought to work - assuming the value from the textbox is ok. If you print the value of cmd.CommandText to the console and then paste it into the SQLite command interpreter, does that work and successfully delete the row(s) ?

I can delete easy from dbBroswer.Not from my program.
I just tried to print the value of pa.Search_Box.Text and guess what. IT'S EMPTY even though I'm referencing to it.My next guess is that i might messed up the order of my code somewhere.

 

[Skydiver]

cmd.CommandText = "DELETE FROM Description WHERE Mould_Code = '"+pa.Search_Box.Text+"'";//Am I missing something here? BTW Mould_Code is PK integer.

Yes, you are missing multiple things there:
1) You should use parameterized queries. If you allow user controlled strings to just be appended into a string, you are at a big risk of a SQL injection action.

https://imgs.xkcd.com/comics/exploits_of_a_mom.png

2) You said that Mould_Code is an integer, but you are trying to compare it against a string value because of the single quotes that you are putting around the value of pa.Searh_Box.Text.

3) As you discovered, you have an empty string for some reason. You shouldn't even be digging out the value of from the UI. Your View or UI layer should be passing in the value to search for into your Delete_Info_From_DB() method. Also ideally, that parameter should be an integer value instead of a string value since you said that the primary key is a integer.

 

[cbreemer]

Valid points from Skydiver. These single quotes should not be there if Mould_code is integer.
Isn't this the same issue as in your other thread Referencing Problem ?
And very strange that the program should 'freeze' when given an empty string. I would have expected either an error message or zero hits.

 

[Kostakis45]

cmd.CommandText = "DELETE FROM Description WHERE Mould_Code = '"+pa.Search_Box.Text+"'";//Am I missing something here? BTW Mould_Code is PK integer.

Yes, you are missing multiple things there:
1) You should use parameterized queries. If you allow user controlled strings to just be appended into a string, you are at a big risk of a SQL injection action.

https://imgs.xkcd.com/comics/exploits_of_a_mom.png

2) You said that Mould_Code is an integer, but you are trying to compare it against a string value because of the single quotes that you are putting around the value of pa.Searh_Box.Text.

3) As you discovered, you have an empty string for some reason. You shouldn't even be digging out the value of from the UI. Your View or UI layer should be passing in the value to search for into your Delete_Info_From_DB() method. Also ideally, that parameter should be an integer value instead of a string value since you said that the primary key is a integer.

1st Answer:I don't really mind of SQL hacking as this is a personal project wich is not going to be in any server.

2nd Answer: Strangely my Search method works fine. It gets a string user input and returns an integer from db.

3d Answer:I tried to do this Delete_Record.Search_Box.Text = Search _Box.Text in my main form.
But it's not working.

 

[Kostakis45]

Valid points from Skydiver. These single quotes should not be there if Mould_code is integer.
Isn't this the same issue as in your other thread Referencing Problem ?
And very strange that the program should 'freeze' when given an empty string. I would have expected either an error message or zero hits.

I was hoping to get errors but it only freezes.

 

[Kostakis45]

So I fixed my references.It's like this now.

using System;
using System.Data.SQLite;
using System.Windows.Forms;

namespace Παράμετροι_Μηχανών
{
    class Delete_Record
    {
        string connection = @"URI=file:" + Application.StartupPath + "\\Injection_Settings.db";
        public static TextBox Search_Box = new TextBox();
        public static TextBox mould_code_input = new TextBox();
        public static TextBox machine_number_input = new TextBox();
        public static TextBox machine_type_input = new TextBox();
        public static TextBox supplier_input = new TextBox();
        public static ComboBox colour_input = new ComboBox();
        public static ComboBox comboBox1 = new ComboBox();
        public static ComboBox comboBox2 = new ComboBox();
        public static ComboBox comboBox3 = new ComboBox();
        public static NumericUpDown numericUpDown1 = new NumericUpDown();
        public static NumericUpDown numericUpDown2 = new NumericUpDown();
        public static NumericUpDown numericUpDown3 = new NumericUpDown();
        public static DateTimePicker dateTimePicker1 = new DateTimePicker();
        public static TextBox item_name_input = new TextBox();
        public static PictureBox pictureBox1 = new PictureBox();
        public static PictureBox pictureBox2 = new PictureBox();

        public void Delete_Info_From_DB()
        {                    
            int val = Int32.Parse(Search_Box.Text);
           
            SQLiteConnection con = new SQLiteConnection(connection);
            con.Open();
            var cmd = new SQLiteCommand(con);
            cmd.CommandText = "DELETE FROM Description WHERE Mould_Code = '"+ val +"'";
            cmd.ExecuteNonQuery();
         
            if ((cmd.CommandText = "SELECT * FROM Description WHERE Mould_Code = "+ val +"") == null)
            {
               MessageBox.Show("Επιτυχής διαγραφή.");

               mould_code_input.Text = null;
               machine_number_input.Text = null;
               machine_type_input.Text = null;
               supplier_input.Text = null;
               colour_input.Text = null;
               comboBox1.Text = null;
               comboBox2.Text = null;
               comboBox3.Text = null;
               numericUpDown1.Text = null;
               numericUpDown2.Text = null;
               numericUpDown3.Text = null;
               dateTimePicker1.Text = null;
               item_name_input.Text = null;
               pictureBox1.Image = null;
               pictureBox2.Image = null;
            }
            else
            {
                    MessageBox.Show("Αποτυχία διαγραφής.");
            }                  
            con.Close();
        }
    }
}

And in my main Form it's like this.

private void Delete_Record_Click(object sender, EventArgs e)
        {
            Delete_Record dr = new Delete_Record();
            Delete_Record.Search_Box = Search_Box;
            Delete_Record.mould_code_input = mould_code_input;
            Delete_Record.machine_number_input = machine_number_input;
            Delete_Record.machine_type_input = machine_type_input;
            Delete_Record.supplier_input = supplier_input;
            Delete_Record.colour_input = colour_input;
            Delete_Record.comboBox1 = comboBox1;
            Delete_Record.comboBox2 = comboBox2;
            Delete_Record.comboBox3 = comboBox3;
            Delete_Record.numericUpDown1 = numericUpDown1;
            Delete_Record.numericUpDown2 = numericUpDown2;
            Delete_Record.numericUpDown3 = numericUpDown3;
            Delete_Record.dateTimePicker1 = dateTimePicker1;
            Delete_Record.item_name_input = item_name_input;
            Delete_Record.pictureBox1 = pictureBox1;
            Delete_Record.pictureBox2 = pictureBox2;

            if (Database_Search.Search_Box != null)
            {
                dr.Delete_Info_From_DB();
            }          
        }

Now I'm getting the user input correctly.I tested it and it print's it.
But the problem still remains.When i click the delete button the program freezes.No errors.I tried without the single quotes and it's the same.

 

[Skydiver]

Break in with the debugger and look at the callstack. That will usually give you a good clue about what is causing the freezing.

 

[Kostakis45]

Break in with the debugger and look at the callstack. That will usually give you a good clue about what is causing the freezing.

After some time it's saying database is locked,but it's not related.I made a new db and it's the same.My guess is tha for some reason it's not reading the user input in this line

cmd.CommandText = "DELETE FROM Description WHERE Mould_Code = '"+ val +"'";

How else i can write this code?

 

[Skydiver]

This may or may not be related to your problem. You said that Mould_Code is an integer, but you still continue to have a condition that compares it against a string. I don't know how SQLite's C# driver would react to that.

Also not directly related to your problem, but that is a terrible design to pass references to your UI controls to the Delete_Record class. You should just pass in the values that need are needed for the deletion. If the class needs the UI to do anything, it should either fire off events or provide callbacks.

 

[Skydiver]

1st Answer:I don't really mind of SQL hacking as this is a personal project wich is not going to be in any server.

It doesn't even have to be in a server. Someday in the future, you are going to be in a rush, and you will just copy and paste the code into a real project. And then nobody is going to try to fix that code because there is always something else more important to do and it becomes a technical debt. And furthermore, some other developers will copy and paste that code elsewhere without fixing it and justify it as: "Our lead dev wrote that code many years ago. It works, and nothing had blown up." And then later there is enough complexity in the code that management gets into the "don't fix what ain't broke" mode of risk management.

 

[Kostakis45]

This may or may not be related to your problem. You said that Mould_Code is an integer, but you still continue to have a condition that compares it against a string. I don't know how SQLite's C# driver would react to that.

Also not directly related to your problem, but that is a terrible design to pass references to your UI controls to the Delete_Record class. You should just pass in the values that need are needed for the deletion. If the class needs the UI to do anything, it should either fire off events or provide callbacks.

For your first paragraph.I don't know any other way to pass a user_input value into sqlite command as val=user_input value.Also i convert the val to integer so I think it should work.If there's another way to pass the val to sqlite command please give me a reference.

For your second paragraph.Do you mean something like that?

public static TextBox Search_Box.Text = new TextBox();

and that?

Delete_Record.Search_Box.Text = Search_Box.Text;

 

[jmcilhinney]

For your first paragraph.I don't know any other way to pass a user_input value into sqlite command as val=user_input value.Also i convert the val to integer so I think it should work.If there's another way to pass the val to sqlite command please give me a reference.

You're not actually reading the words being posted. You've been told twice now that the specific issue is the single-quotes. No one is telling you that you need to pass the data a different way. They're telling you that you don't wrap numbers in single quotes, so don't. In your C# code, do you put double-quotes around numbers? Of course you don't. That's just for strings. Why, then, do you insist on wrapping numbers in single-quotes in your SQL code? Single-quotes are just for strings in SQL, not numbers.

 

[jmcilhinney]

For your second paragraph.Do you mean something like that?

public static TextBox Search_Box.Text = new TextBox();

and that?

Delete_Record.Search_Box.Text = Search_Box.Text;

No. Your Delete_Record class should not know anything about any controls at all. It should be concerned only with data. If you want to save data then just pass in the data and save it. If you want to delete data then it should be the responsibility of the form itself to clear the controls after a successful delete. If your extra class is going to do the UI work as well then that class is pointless and you may as well do the data work in the form. If you're going to create extra classes to do other jobs then they should ONLY do those jobs. The form does the UI stuff and the data class does the data stuff. Never the twain shall meet.