Exception handling in DAL

Question

In a MVP winforms application I'm handling exceptions as follows in DAL.

Since the user messaging is not a responsibility of DAL, I want to move it in to my Presentation class.

Could you show me a standard way to do that?

    public bool InsertAccount(IBankAccount ba)
    {
        string selectStatement = @"IF NOT EXISTS (SELECT ac_no FROM BankAccount WHERE ac_no=@ac_no) BEGIN INSERT INTO BankAccount ...";

        using (SqlConnection sqlConnection = new SqlConnection(db.ConnectionString))
        {
            using (SqlCommand sqlCommand = new SqlCommand(selectStatement, sqlConnection))
            {
                try
                {
                    sqlConnection.Open();
                    sqlCommand.Parameters.Add("@ac_no", SqlDbType.Char).Value = ba.AccountNumber;
                    //
                    //

                    sqlCommand.ExecuteNonQuery();
                    return true;
                }
                catch (Exception e) { MessageBox.Show(("Error: " + e.Message)); }
                if (sqlConnection.State == System.Data.ConnectionState.Open) sqlConnection.Close();
                return false;
            }

        }
    }

EDIT2 :

So based on the answers I re edited the post and now my exception handling code looks like this...

DAL

public bool InsertAccount(IBankAccount ba)
{
    try
    {
        sqlConnection.Open();
        //   
    }
    catch (SqlException)
    {
        throw new Exception("DataBase related Exception occurred");
    }
    catch (Exception)
    {
        throw new Exception("Exception occurred");
    }
}

BankAccountPresenter

    private void SaveBankAccount()
    {
        try
        {
           _DataService.InsertAccount(_model);
        }
        catch (Exception e) { MessagingService.ShowErrorMessage(e.Message); }
    }

Why I've caught exceptions in DAL is that even at the moment I'm not logging errors, I may have to do it in future.

And also this way I can differentiate the error massages in DAL, whether it's sql related or general.

I've used messaging service in my presenter when showing error messages.

Does this meaning full? Can this be simplified?

FYI, when wrapping a connection in a using there is no need to close the connection. The using will automatically close the connection even if there is an exception. See [this](http://stackoverflow.com/questions/4717789/in-a-using-block-is-a-sqlconnection-closed-on-return-or-exception) question. — Aaron Carlson, Jun 13 '14 at 12:37
Looking at your edit. It's not the best idea to catch `Exception` just to throw a new `Exception` with the "Invalid Bank Account" message. You should only catch what you can handle. Something like an `OutOfMemoryException` isn't an invalid bank account. — Neil Smith, Jun 13 '14 at 13:40
@Smith.h.Neil, Ok, If my objective is, when some thing goes wrong, alert the user and keep the system up (with out crashing), the way I've handled not enough? If that is the case, should I re throw exception? — CAD, Jun 13 '14 at 13:46
Well sometimes the app is going to crash and that's all there is to do. If it runs out of memory, it's out of memory. I actually wouldn't catch in your DAL (at least not from what I've seen). You don't handle anything there so there's no reason to catch there. In your presenter, I'd catch `SqlException` and properly handle (display error message) to user and I'd let critical exceptions be handled by the clr. — Neil Smith, Jun 13 '14 at 14:00

Nathan Cooper · Accepted Answer · 2014-06-13T19:28:20.827

5

You're returning false to indicate that there has been an exception. This is not recommended.

Rethrow it

catch(Exception e) {
  //blah, maybe add some useful text to e
   throw;
}
finally { //close up shop Although, look up what using does first incidentally }

Then deal with it (catch (Exception e) { MessageBox.Show(("Error: " + e.Message)); }) in a higher level.

Reponse to EDIT2:

That's fine. However, your current implementation throws the 'actual' exception and its stack trace in the bin in favour of your helpful message. You need to add it as an inner exception like this:

catch(Exception e){    
     throw new Exception("some helpful message", e);
}

edited Jun 13 '14 at 19:28

answered Jun 13 '14 at 12:29

Nathan Cooper

6,262
4
36
75

2

Indeed, special remark to use 'throw;' instead of 'throw ex;' to keep stack trace. – L-Four Jun 13 '14 at 12:33
@L-Three That's why I posted code. Maybe it should have been explicit. It's a trap for sure. throw e will reset paramters, such as the Exception.StackWalk – Nathan Cooper Jun 13 '14 at 12:34
In a higher level means, in the presenter or in side the same DAL? – CAD Jun 13 '14 at 12:36
1

@Chaturanga, Presentation layer. You only catch it for good when you're good and ready to deal with it, in your case by alerting the user. If its something you can deal with in the DAL, by trying again for example, obviously that isn't the case. – Nathan Cooper Jun 13 '14 at 12:50
@ Nathan Cooper, I re edited the post. Now I'm handling the exceptions thrown in Presenter. See EDIT2: seciton if you don't mind. – CAD Jun 13 '14 at 17:32
1

@Chathuranga Replied. I think your implementation is just fine. I'm not sure why `sqlConnection.Open();` is there. I think you still need to look up [`using`](http://www.codeproject.com/Articles/6564/Understanding-the-using-statement-in-C). It's short to write, clarifies your intentions (making it very clear when the connection is being used and in scope) and saves you from mistakes (I notice your dispose was in the `try`, not in a `finally`. That would be a mistake, but you don't need to care if you were using `using`) – Nathan Cooper Jun 13 '14 at 19:36
Whoops, looking back. Of course `sqlConnection.Open();` still needs to be called in a using block. – Nathan Cooper Jul 16 '14 at 10:35
This: `..throws the 'actual' exception and its stack trace in the bin in favour of your helpful message..` Is a very good point. Thank you! – BendEg Feb 03 '15 at 09:28

Exception handling in DAL

1 Answers1