views:

217

answers:

3

I have struggled with this since day 1. It probably doesn't help that I've been surrounded by a lot of code that doesn't even handle errors at all.

Anyway, I'm working with WebForms in your traditional n-tier design: UI->BLL->DAL. Usually what I do (and I know it's not right) is to try/catch my data operations. If there is an exception I simply throw it to bubble up.

try
'db operations
catch ex as exception
throw
finally
'close connections
end

So then it bubbles up to the BLL and in there is another try/catch where I'll log the error. Now I want to alert the user that something is wrong so I throw it again, this way it bubbles to the UI. At the UI level, I will wrap in a try/catch and if there's an error I'll display a friendly message to them.

What are your thoughts? What can I do better here?

+5  A: 

As I understand, you have three try/catches - one for each tier: DAL, BLL,UI.

You should only catch when you are going to do something about it.

From what I understand, you simply retrow from DAL so there's no need for it.

From BLL you log the exeption and that is a good practice since you'll be able to collect data about exceptions to eventually improve the application.

Then you catch at the UI tier to translate the exception into something user friendly. That's OK.

So I would only get rid of the try/catch from the DAL layer - if you really don't do anything more than retrowning the exception.

In some scenarios, it can be usefull to add an identifier at the BLL that is passed to the UI exception and shown to the end users so that if they call support, support personel can correlate the given Id with an exception on the server's log.

That can be done, for instance, adding a Guid or something else meaningfull and unique to Exception.Data collection.

Alfred Myers
But if an error occurs in the DAL, don't have I have to close the connection to prevent leaks?
Beavis
just put your connection/command objects in a using{} block
Binoj Antony
There's no need to **explicitly** try/catch on DAL if you are using the "Using" statement. Under the covers, "Using" will wrap your connection/command with a try/finally and close them in the finally.
Alfred Myers
A: 

you need to throw the exception and catch in UI, Like... In BLL

try
        {
            //your code
        }
        catch (System.Data.SqlClient.SqlException ex)
        {
            if (ex.Number == 547)
            {
                throw new Exception("ActiveRecord");
            }
        }
        finally
        {
            MasterConnection.Close();
        }

Catech the execption in UI and show user friendly message.

 if (e.Exception != null)
    {
        if (e.Exception.InnerException.Message == "ActiveRecord")
        {
            //Show here User freind message
        }
    }
Muhammad Akhtar
A: 

Unlike the other answers, I am going to advise removing the try/catch from your UI as well. Your BLL should catch the exception, log it and then provide some mechanism for the UI to query what errors occurred.

This results in much cleaner UI code, as you do not have to enumerate all the possible exceptions that the business layer may throw. You can simplify your UI code to:

if (businessObject.doSomeOperation == true) {
   // Do whatever you need to here.
} else {
   // output error message from businessobject
   // something like businessObject.LastErrorMessage();
}
Jason Berkan