CODEWITHSUNDEEP
c#.netsql-server-2008
user450143
user450143

Reputation: 435

SQL Connection code

Here's my code for a SQL connection. Not sure if I have handled all errors. Instead of Using should I just use Try Catch block?

using (SqlConnection con = new SqlConnection(GetSQLConnectionString()))
{
    string cmdStr = "INSERT INTO table1.....";

    using (SqlCommand cmd = new SqlCommand(cmdStr, con))
    {
        //Store parameters with values to the collection
        cmd.Parameters.AddWithValue("NAME", NAME);
        cmd.Parameters.AddWithValue("ID",ID);

        try
        {
            con.Open();
            cmd.ExecuteNonQuery();

        }
        catch (Exception ex)
        {
            lblError.Text = ex.ToString();
        }

        if (con != null)
           con.Close();
    }
}

Upvotes: 0

Views: 304

Answers (3)

Damien_The_Unbeliever
Damien_The_Unbeliever

Reputation: 239636

I'd re-write your current catch block to only catch SQLException - catching any Exception is rarely correct. This would also mean that you could produce prettier output (formatting each separate SQLError object), include a total error count, or react to specific errors that you know your code can handle.

A general rule for exception handling is to only catch exceptions that your code can correctly deal with at that point (hence the general avoidance of catching Exception). You'd generally install a single top level "unhandled Exception" handler in your app that will log other exceptions and shut down the app. If you don't know what the exception is, how can you hold a belief that your code can continue to operate successfully when it has occurred?

As Neil says, you don't need to explicitly close the connection since the using block guarantees this as well.

Upvotes: 0

Marc Gravell
Marc Gravell

Reputation: 1062540

Having a catch block from data-access talk to lblError tells me you have your UI code and your DB code too close together. I would not do that; my DB code would be simply:

using (var con = SomeUtilityCode.GetOpenConnection())
{
    string cmdStr = "INSERT INTO table1.....";

    using (SqlCommand cmd = new SqlCommand(cmdStr, con))
    {
        //Store parameters with values to the collection
        cmd.Parameters.AddWithValue("NAME", name);
        cmd.Parameters.AddWithValue("ID",id);

        cmd.ExecuteNonQuery();
    }
}

or perhaps something a lot cleaner using "dapper":

using (var con = SomeUtilityCode.GetOpenConnection())
{
    con.Execute("INSERT INTO table1.....", new { NAME = name, ID = id });
}

and my UI code would be

try
{
   someObject.SomeSensibleMethod(NAME, ID);
}
catch (Exception ex)
{
   ShowError(ex);
}

where ShowError tells the user about the problem (perhaps sanitized) without needing evert operation to know the UI details.

Upvotes: 2

Neil Knight
Neil Knight

Reputation: 48537

The try/catch block is a good idea as the using won't catch any errors, it will just throw the exception and stop.

Also, in your using code, you don't need to do:

if (con != null)
    con.Close();

as this will be handled by the using statement.

Upvotes: 1

Related Questions