.Net: What is your confident approach in "Catch" section of try-catch block, When developing CRUD operations?

Question

I was wondering if there would be any confident approach for use in catch section of try-catch block when developing CRUD operations(specially when you use a Database as your data source) in .Net?

well, what's your opinion about below lines?

public int Insert(string name, Int32 employeeID, string createDate)
    {
        SqlConnection connection = new SqlConnection();
        connection.ConnectionString = this._ConnectionString;
        try
        {
            SqlCommand command = connection.CreateCommand();
            command.CommandType = CommandType.StoredProcedure;
            command.CommandText = "UnitInsert";
            if (connection.State != ConnectionState.Open)
                connection.Open();
            SqlCommandBuilder.DeriveParameters(command);
            command.Parameters["@Name"].Value = name;
            command.Parameters["@EmployeeID"].Value = employeeID;
            command.Parameters["@CreateDate"].Value = createDate;
            int i = command.ExecuteNonQuery();
            command.Dispose();
            return i;
        }
        catch
        {
            **// how do you "catch" any possible error here?**
            return 0;
            //
        }
        finally
        {
            connection.Close();
            connection.Dispose();
            connection = null;
        }
    }

Totally on a tangent, but you should try using `using` statements, rather than `try`/`finally` and calling `Dispose` yourself. http://msdn.microsoft.com/en-us/library/yh598w02(VS.80).aspx — Matthew Scharley, Apr 29 '10 at 12:02
Why do you check if the connection state is open? It will not be open, so just call `Open` without the previous if. — Klaus Byskov Pedersen, Apr 29 '10 at 12:03
A tangent is a line that touches a point on a curve, but doesn't actually intersect (cross) it. To go off on a tangent is to touch on the original issue, but continue on your way without actually addressing it. Using `using` will help clean up your code, but doesn't really answer your question. — Matthew Scharley, Apr 29 '10 at 22:10

kemiller2002 · Accepted Answer · 2010-04-29T12:28:18.010

I would use a using statement for starters.

I wouldn't return 0 as a failure. You can successfully update no records and so 0 would be a valid success response code. Using -1 clearly shows that something went wrong. Personally, I would much rather throw an exception in the event something enexpected happened.

try
    { 
       using (SqlConnection connection = new SqlConnection())
        {

            connection.Open();         

            using(SqlCommand command = connection.CreateCommand())
            {
                    command.CommandType = CommandType.StoredProcedure;
                    command.CommandText = "UnitInsert";

                    SqlCommandBuilder.DeriveParameters(command);
                    command.Parameters["@Name"].Value = name;
                    command.Parameters["@EmployeeID"].Value = employeeID;
                    command.Parameters["@CreateDate"].Value = createDate;

                    return command.ExecuteNonQuery();
               }

        }
        }
        catch(Exception ex)
        {
          LogException(ex);
           return either -1 or re throw exception.
        }

+1. Also, I would throw a custom exception, such as `throw new MyCustomException("Failed to create employee", ex);` — Klaus Byskov Pedersen, Apr 29 '10 at 12:05
@odiseh you could have a catch block where you don't catch the exception, but then you have no idea what went wrong. I always make it a rule that you have to do something with the exception. Even if it is catch a particular exception and ignore it. — kemiller2002, Apr 29 '10 at 12:30

score 2 · Answer 2 · answered Apr 29 '10 at 12:02

In my opinion, this is wholly the wrong place to catch anything that you can't handle there and then. Let the exception bubble up and have the calling application implement it's own error handling. If you catch and swallow an exception here, your debugging for installed applications is going to be nightmarish.

Only ever catch what you can handle and throw everything else...

score 0 · Answer 3 · answered Apr 29 '10 at 12:02

0

I would do a

catch(Exception ex){
     // log exception here and set return value
}

answered Apr 29 '10 at 12:02

derek

4,826
1
23
26

score 0 · Answer 4 · answered Apr 29 '10 at 12:08

I like to formalize my DAL's API by contracting all exceptions to a select few, and nesting the actual exception as an InnerException.

For example, all database exceptions that aren't the callers fault would throw one type of exception, all database exception that are the callers fault (such as no rows selected, invalid PK, invalid data) would throw another type of exception (or perhaps even have a finer grained distinction between exception types), and one last exception type for stuff that is not related to the database (sanity checks, NullRef, etc), except for exceptions that I can't handle (such as OutOfMemoryException).

That way it is easy to catch the exceptions I throw in my DAL, and all the specific gory details are still available in InnerException.

.Net: What is your confident approach in "Catch" section of try-catch block, When developing CRUD operations?

4 Answers4