1

I have the following scenario where I call a method in my data-access code from my business-layer:

//Call the method from BL
SqlHandler sh = new SqlHandler();
var names = sh.GetNames();

Method example in DAL:

public IEnumerable<string> GetNames()
{
    string conString = GetOpenConnection();
    using (SqlConnection connection = new SqlConnection(conString))
    {
        connection.Open();
        using(SqlCommand cmd = new SqlCommand("SELECT Name From Names"))
        {
            //...
        }
    }
}

My need is to log any exception that occurs in my DAL and display a message to user with an appropriate message. As far as I can see I have the following options:

1) Surround only the BL call and log and display the message from there:

try
{
    SqlHandler sh = new SqlHandler();
    var names = sh.GetNames();
}
catch (Exception ex)
{
    ex.LogException();
    MessageBox.Show(ex.Message);
}

2) Surround both calls and split the logging part and the notifying part into 2 sections:

try
{
    SqlHandler sh = new SqlHandler();
    var names = sh.GetNames();
}
catch (Exception ex)
{
    MessageBox.Show(ex.Message);
}

public IEnumerable<string> GetNames()
{
    try
    {
        string conString = GetOpenConnection();
        using (SqlConnection connection = new SqlConnection(conString))
        {
            connection.Open();
            using (SqlCommand cmd = new SqlCommand("SELECT Name From Names"))
            {
                //...
            }
        }
    }
    catch (Exception ex)
    {
        ex.LogException();
        //propagate the exception to the caller
        throw;
    }
}

3) of course I might be missing something here that I would be happy to know about

What is the preferred approach in terms of application architecture here? Is there a functional difference between the top two?

Yoav
  • 3,326
  • 3
  • 32
  • 73

2 Answers2

2

I'd suggest a third option: Put error logging and error display in the UI layer.

Register a central error handler in your user interface. There, you can

  • display the error (either a simple MessageBox or a fancy window with a "more details" and a "report to developers" button) and

  • log everything,

without having to clutter every BL method with catch-all try-catch clauses.

This has a lot of advantages:

  • It will make your BL code easier to read.
  • You cannot forget a BL method.
  • It's easier to make (centralized) changes to your error handling code.
  • You do not couple your business logic to a particular type of user interface, which has a lot of advantages on its own: For example, you can reuse it in a web-based project later, and you can test your business logic with unit tests.
Community
  • 1
  • 1
Heinzi
  • 167,459
  • 57
  • 363
  • 519
  • This is an interesting approach however, how could I know after a call to an exception prone method if it succeeded or not (_i.e_ should I retry the call or stop executing the rest of the code and return)? – Yoav Jun 21 '15 at 14:37
  • @Yoav: *Ususally*, exceptions are something unexpected that you cannot reasonably recover from. I would suggest to handle special cases on a case-by-case basis: If you have a very specific method of which you know that it sometimes throws a very specific exception (and where you know what to do against it, e.g. retry), then I would wrap *this* (and only this) method in a try-catch block that catches *this* (and only this) exception (i.e. `catch (MyExpectedDeadlockException)` rather than `catch (Exception)`) and does something specific for this case. – Heinzi Jun 21 '15 at 16:52
  • @Yoav: For all other types of exception, interrupting the BL operation, logging, showing a message and returning to the UI is the right thing to do. And that's exactly what this approach would do. – Heinzi Jun 21 '15 at 16:54
0

First of all : Always catch only exceptions you can handle (See Eric Lippert Post)
You can follow this strategy :

  • If you cannot recover from the exception do it in the DAL layer only and log it.
  • try {} catch{} in BAL layer if the user can handle it.
hdoghmen
  • 3,175
  • 4
  • 29
  • 33
  • I do not fully agree with the perception on only catching exceptions that you can handle. there are some types of exceptions that you cannot expect or it's not worth the time to try and predict such as DB related or TCP related exceptions that you will still want to let the application \ user to retry before throwing – Yoav Jun 21 '15 at 14:26
  • @Yoav If you have unexpected exceptions, let them crash your process. That way, you'll be aware that they exist and force you to handle them, instead of staying logged in some txt file – Yuval Itzchakov Jun 21 '15 at 14:36
  • @YuvalItzchakov so would you suggest not surrounding a DB operation at all (and lets leave transactions aside of course) ? – Yoav Jun 21 '15 at 14:40
  • Recover from what you can. Fix as you go. – Yuval Itzchakov Jun 21 '15 at 14:41
  • @YuvalItzchakov could you please give me an example of a recoverable exception? – Yoav Jun 21 '15 at 14:50
  • @Yoav Lets say you use EF and which you know (by documentation) may throw throw a `DbException` when calling `Save()`. That is an exception you can recover from, perhaps with a retry mechanism. – Yuval Itzchakov Jun 21 '15 at 14:53
  • `IndexOutOfRangeException` is a recoverable exception : as mentionned in the blog link below : *Boneheaded exceptions are your own darn fault, you could have prevented them and therefore they are bugs in your code* – hdoghmen Jun 21 '15 at 14:53