views:

54

answers:

4

hi,

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;
        }
    }
+3  A: 

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.
        }
Kevin
+1. Also, I would throw a custom exception, such as `throw new MyCustomException("Failed to create employee", ex);`
klausbyskov
@klausbyskov that's also a good point.
Kevin
@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.
Kevin
+1  A: 

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...

Paddy
A: 

I would do a

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

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.

Allon Guralnek