views:

115

answers:

2

Our code catches the general exception everywhere.

Usually it writes the error to a log table in the database and shows a MessageBox to the user to say that the operation requested failed. If there is database interaction, the transaction is rolled back.

I have introduced a business logic layer and a data access layer to unravel some of the logic. In the data access layer, I have chosen not to catch anything and I also throw ArgumentNullExceptions and ArgumentOutOfRangeExceptions so that the message passed up the stack does not come straight from the database.

In the business logic layer I put a try catch. In the catch I rollback the transaction, do the logging and rethrow.

In the presentation layer there is another try catch that displays a MessageBox.

I am now thinking about catching a DataException and an ArgumentException instead of an Exception where I know the code only accesses a database.

Where the code accesses a web service, then I thought I would create my own "WebServiceException", which would be created in the data access layer whenever an HttpException, WebException or SoapException is thrown.

So now, generally I will be catching 2 or 3 exceptions where currently I catch just the general Exception, and I think that seems OK to me. Does anyone wrap exceptions up again to carry the message up to the presentation layer?

I think I should probably add a try catch to Main() that catches Exception, attempts to log it, displays an "Application has encountered an error" message and exits the application. So, my question is, does anyone see any holes in my plan? Are there any obvious exceptions that I should be catching or do these ones pretty much cover it (other than file access - I think there is only 1 place where we read-write to a config file).

+2  A: 

Seems like you are making some good improvements to your application. I also generally wrap the most outer most layer of an application in a try/catch Exception block so that unexpected exceptions (e.g. runtime ones) can allow the application to exit gracefully.

One question for you is if it makes sense to delegate to the business logic the responsibility of a data rollback. This would seem better encapsulated in the data access layer. You could then throw an exception of your choice from the data layer rather than opening up that layer to throwing any old exception, because what happens to your business logic layer if your data layer throws something you didn't expect?

Or what if you later extract your data access layer and replace it with something else? At the very least, you'll have to remove all your rollback logic from the business logic layer.

Chris Knight
Thank-you Chris. I have considered that dilemma. There are methods in the business logic that combine data access calls that should probably be entirely in the data access layer. On the other hand there are some methods that combine data calls that make sense to me to be business logic.To get around that I call MyDatabase.BeginTransaction() on the DAL and MyDatabase.RollbackTransaction(). I am sure there are better ways of handling "Business" transactions, but at least the BLL is not dependent on a SqlCeTransaction.....
Colin
+2  A: 

I would use try/finally (or the using statement which is equivalent) for your rollback, rather than doing it in a catch block. In particular if you are only catching specific exceptions, you will still want the database rollback to take place if an unexpected Exception type is thrown.

With few exceptions I rarely use a catch anywhere except:

  • At a physical tier boundary I use try/catch with log/throw in the catch block so that exceptions can be logged on the server. With newer technologies such as WCF exceptions can be logged without a try/catch (e.g. with a WCF DispatchBehavior).

  • In a top-level handler in the presentation tier.

In the business and data tiers, there will be a lot of try/finally (i.e. using statements) but almost never a catch.

Joe
Excellent point about finally or using statement. I already do that where the transaction is in the DAL, but some of my transactions are still in the BLL, so I guess I will need to expose a MyDatabase.Dispose() method, or wrap my SqlCeTransaction for the "using" statement
Colin
Not sure what you mean by a physical boundary. Isn't there a boundary at every method call from BLL to DAL? In that case every call in the BLL should have the try/catch log/throw pattern.But if it means the physical boundary between application and database, or application and web service, or application and file system, then I would move my logging to the DAL not the BLL.But you say you have almost no catches in the DAL or BLL, and of course I've just twigged that the catches should sit in the "MyDatabase" class where all updates inserts and deletes are done. That does sound much better.
Colin