"Exception Unhadled" error during SqlConnection open()

Question

In my program I have two text input fields (in windows form) and a button to add/save those values into the DB table. The problem is once I click the button, it does not insert the inputs into the DB, instead it shows the error I have attached below as image.

What is wrong with my program?

My working code:

public partial class Form1 : Form
{
    //original Connection string is exactly the following:
    //Data Source=(LocalDB)\MSSQLLocalDB;AttachDbFilename="C:\Users\Sanad Al Nahaj\Documents\thesaurus.mdf";Integrated Security=True;Connect Timeout=30
    SqlConnection conn = new SqlConnection(@"Data Source = (LocalDB)\MSSQLLocalDB; AttachDbFilename=C:\Users\Sanad Al Nahaj\Documents\thesaurus.mdf;Integrated Security = True; Connect Timeout = 30");
    public Form1()
    {
        InitializeComponent();
    }

    //Save button
    private void button1_Click(object sender, EventArgs e)
    {
        conn.Open();//error pops up here after clicking the button
        SqlCommand command = conn.CreateCommand();
        command.CommandType = CommandType.Text;
        command.CommandText = "insert into Table values('"+wordbox.Text+"','"+synonymbox.Text+"')";
        command.ExecuteNonQuery();
        conn.Close();
        MessageBox.Show("Word and Synonym added!");
    }

    private void display_Click(object sender, EventArgs e)
    {
        //implement
    }
}

Error:

Database looks like:

UPDATE: My modification in Using pattern (referring to CDove's answer):

    var command = new SqlCommand();
            using (command = new SqlCommand(
                          "insert into......)
))

I think I see a issue: you are not properly disposing of the Connection. This whole pattern of keeping the connection open as a class scale variable is a bad idea. Create. Use. Dispose. It is the only way to handle disposeable stuff (https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/using-statement). Do not worry about performance. SQLConenction itself might do some connection pooling. And in teh end a fault free running programm takes precedence. — Christopher, Feb 07 '18 at 16:30
Could you please try to do: in your web.config file and try again. — Eray Balkanli, Feb 07 '18 at 16:30
Eray Balkanli, do you mean in my web.config file? in that case, should it be inside tag? — Sanad, Feb 07 '18 at 16:36
I mean the web.config file for the related application. I believe you will see it under the project in Visual Studio — Eray Balkanli, Feb 07 '18 at 16:39
It should be inside system.web like: If not, you can try to add it like here. — Eray Balkanli, Feb 07 '18 at 16:40
Eray Balkanli , I guess you already should know that its an windows application. not a web application. So it has the app.config file instead of web.config — Sanad, Feb 07 '18 at 16:41
Make sure you are using the `using` statement. If an exception occurs after the connection is open, it isn't automatically closed and you have unhandled an external resources not cleaned up. — Erik Philips, Feb 07 '18 at 16:45

CDove · Accepted Answer · 2018-02-08T13:33:29.477

5

There are four things you need to do. First, address this:

"insert into [Table] values('"+wordbox.Text+"','"+synonymbox.Text+"')"

In Microsoft SQL, if I recall correctly, values() syntax on an insert requires explicit declaration of the columns first. Also, "Table" is a reserved word, so you'll need to put it in brackets to use the word as a table name. In the future, avoid using reserved words in your table schemae.

"insert into [Table] (word, synonym) values ('"+wordbox.Text+"','"+synonymbox.Text+"')"

Second, don't use string concatenation to build a query. Create parameters instead.

"insert into [Table] (word, synonym) values (@word,@syn)"

And then

 command.Parameters.AddWithValue("@word", wordbox.Text); 
 command.Parameters.AddWithValue("@syn", synonymbox.Text);
 command.ExecuteNonQuery();

Third, don't cache your connection. That's what this does at the top of your code, leaving you one connection you have to micromanage:

SqlConnection conn = new SqlConnection(@"Data Source = (LocalDB)\MSSQLLocalDB; AttachDbFilename=C:\Users\Sanad Al Nahaj\Documents\thesaurus.mdf;Integrated Security = True; Connect Timeout = 30");

While ideally, you'd read this from web.config or app.config, we'll roll with your hardcoded string; leave it just a string.

string conn = @"Data Source = (LocalDB)\MSSQLLocalDB; AttachDbFilename=C:\Users\Sanad Al Nahaj\Documents\thesaurus.mdf;Integrated Security = True; Connect Timeout = 30";

Finally, use the using pattern. This is not only less smelly code, but it also includes implicit .Close() and .Dispose() in an implicit try-finally manner.

private void button1_Click(object sender, EventArgs e)
{
    using(var command = new SqlCommand(
             "insert into [Table] (word, synonym) values (@word,@syn)",
              new SqlConnection(conn)
          ))
    {
       command.Connection.Open();//Since we aren't reopening an old connection, errors are less likely.    
       command.CommandType = CommandType.Text;
       command.Parameters.AddWithValue("@word", wordbox.Text); 
       command.Parameters.AddWithValue("@syn", synonymbox.Text);                   

       if(command.ExecuteNonQuery() > 0 )
           MessageBox.Show("Word and Synonym added!");
    }
}

Note that I check the value of ExecuteNonQuery. That's because this function returns the number of rows affected; if the count is 0, the word and synonym weren't added.

Note: This is all off the top of my head on my lunchbreak so test it for yourself, see how it works out for you.

edited Feb 08 '18 at 13:33

answered Feb 07 '18 at 17:14

CDove

1,940
10
19

Table is a reserved word, I believe, so it would need to be in brackets. – LarsTech Feb 07 '18 at 17:20
Good call. I'll add it to the answer. – CDove Feb 07 '18 at 17:26
Thanks for your answer, but I am not clear about the "using" pattern. Should the whole block of code go inside the button_click method? Could you please format and write the whole corrected code at once, so that I don't get confused. – Sanad Feb 07 '18 at 17:42
`usings` go around the block of code specifically using that object. They are a way to use objects inheriting `IDisposable`. In the example, you would just wrap your code that calls the database in the `using` block, and declare your command object that way as shown. In this case, it really does all go inside the `button_click` event handler. You probably want the string with the connection at the class level where you already had it as an `SqlConnection`so you can access the string throughout the class. – CDove Feb 07 '18 at 17:47
I put it inside your handler for clarity. The connection string (conn) *could* be local here, but you would typically want that coming from a class level variable (i.e. declared outside a method) or something reading the config file (separate method). – CDove Feb 07 '18 at 17:49
CDove, are you missing a semicolone after "))"?? – Sanad Feb 07 '18 at 17:55
`In Microsoft SQL, if I recall correctly, values() syntax on an insert requires explicit declaration of the columns first.` thats not true, if your values has all the data you can ommit the column names - but if your table has 10 columns and you only give less than 10 values you have to specify the columns. – Rand Random Feb 07 '18 at 18:13
1

Another thing... - I doubt that your using will close the connection to database. - You are only disposing the SqlCommand but never the connection, I doubt that disposing the SqlCommand will dispose the SqlConnection since there is no way for SqlCommand to know if the SqlConnection gets passed into the constructor like you did or if its a variable outside the scope. --- I would suggest the following syntax `using (var sqlConnection = new SqlConnection(con)) { sqlConnection.Open(); using (var sqlCommand = sqlConnection.CreateCommand) { //do stuff } }` – Rand Random Feb 07 '18 at 18:18
I am super confused with everything now. Could anyone please put the whole class with corrected version? I will very much appreciate that. – Sanad Feb 07 '18 at 18:37
@RandRandom : When a `using` collapses, it calls `Dispose()`, which then calls `Dispose()` on anything inheriting IDisposable() that is a managed resource for that `IDisposable()` object (if it's done properly). In the case of `SqlConnections`, it works either way; a database connection which doesn't exist is as closed as a database connection which is closed before it ceases to exist. It doesn't leave an open hook to the database. – CDove Feb 07 '18 at 18:56
@Sanad , you don't use a semicolon after declaring a `using`, because the next actions are either nested inside with brackets, or follow immediately. Other examples of bracket use/implicit nests like this would be `if` statements, `while` loops, and object Properties with `get` and `set` methods. – CDove Feb 07 '18 at 18:59
@Sanad I added the solution to your question so you can see it in context. I also capitalized your method names, which is a standard convention (VS 2017 will actually throw warnings if you start a method name with a lowercase letter). – CDove Feb 07 '18 at 19:31
CDove, I appreciate your effort to solve my issues. Thanks for that. The using pattern work after I have done some modification (see my update in code section). Is that okay the way I have initialized the variable outside "using"? Otherwise if I keep the "var SqlCommand" declaration inside the "using" as you have suggested, it shows the error: "implicitly typed variables must be initialized". – Sanad Feb 07 '18 at 22:42
That's a typo. It should just be var or SqlCommand. I'll fix it in the answer. – CDove Feb 08 '18 at 13:33

"Exception Unhadled" error during SqlConnection open()

1 Answers1