Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

SQL delete command?

Tags:

c#

sql

sql-delete

I am having trouble with a simple DELETE statement in SQL with unexpected results , it seems to add the word to the list??. Must be something silly!. but i cannot see it , tried it a few different ways. All the same result so quite confused.

public void IncludeWord(string word)
{
    // Add selected word to exclude list
    SqlConnection conn = new SqlConnection();
    String ConnectionString = "Data Source = dev\\SQLEXPRESS ;" + "Initial Catalog=sml;" + "User id=** ;" + "Password =*;" + "Trusted_Connection=No";

    using (SqlConnection sc = new SqlConnection(ConnectionString))
    {
        try
        {
            sc.Open();

            SqlCommand Command = new SqlCommand(
               "DELETE FROM excludes WHERE word='@word'" +
                 conn);


           Command.Parameters.AddWithValue("@word", word);  
            Command.ExecuteNonQuery();
        }
        catch (Exception e)
        {
            Box.Text = "SQL error" + e;
        }
        finally
        {
           sc.Close();
        }
        ExcludeTxtbox.Text = "";

       Box.Text = " Word : " + word + " has been removed from the Exclude List";

        ExcludeLstBox.AppendDataBoundItems = false;
        ExcludeLstBox.DataBind();
    }
like image 941
user685590 Avatar asked Jun 20 '11 15:06

user685590


1 Answers

Try removing the single quotes. Also why are you concatenating your SQL string with a connection object (.. word='@word'" + conn)???

Try like this:

try
{
    using (var sc = new SqlConnection(ConnectionString))
    using (var cmd = sc.CreateCommand())
    {
        sc.Open();
        cmd.CommandText = "DELETE FROM excludes WHERE word = @word";
        cmd.Parameters.AddWithValue("@word", word);  
        cmd.ExecuteNonQuery();
    }
}
catch (Exception e)
{
    Box.Text = "SQL error" + e;
}
...

Notice also that because the connection is wrapped in a using block you don't need to Close it in a finally statement. The Dispose method will automatically call the .Close method which will return the connection to the ADO.NET connection pool so that it can be reused.

Another remark is that this IncludeWord method does far to many things. It sends SQL queries to delete records, it updates some textboxes on the GUI and it binds some lists => methods like this should be split in separate so that each method has its own specific responsibility. Otherwise this code is simply a nightmare in terms of maintenance. I would very strongly recommend you to write methods that do only a single specific task, otherwise the code quickly becomes a complete mess.

like image 60
Darin Dimitrov Avatar answered Sep 24 '22 15:09

Darin Dimitrov