Exception Hierarchy

CodingYoshi

Lets say I have a class called Person (Encapsulates the business layer functionality) and PersonManager (Encapsulates Data Access Layer). Here are code snippets simplified for clarity: public class Person { public void Save() { Personmanager manager = new PersonManager(); manager.Save(this); } } public class PersonManager { public void Save(Person p) { if (p.IsNew) Insert(p) // Private method else { if (P.IsOld) Update(p) // Private Method } } } During insertion problems can occur so Insert method should deal with it and if it can not then it should throw it to the caller. Ideally, Insert should not reveal its inner working to the caller and should not break encapsulation but let the caller know something exceptional happened. The PersonManager class will also try to deal with it, and if it can not, throw it to the caller (Person) without breaking encapsulation. The Person class will follow the same rule. My questions are: 1. Why do we append InnerExceptions as they will break encapsulation? For example, if an SqlException occurs and I attach it as inner exception, now the caller knows I am dealing with SQL database and encapsulation is broken! If I do not attach it then caller will not have sufficient enough information. 2. GUI will have a try-cach which calls person.Save() and Person will have a try-catch which calls PersonManager.Save(Person p) and Insert() and Update will also need try-catch blocks. Am I right? Is this nesting too far or is this how things should be done ideally? 3. Every class, and the method within it should try to deal with the exception and try an alternative or retry again. Is there a general rule of thumb how many times it should try and give up? Does it depend on how critical the system is? Am I the only one who is lost because I have read many sources to obtain these answers? Please provide any useful links or book names.

Luc Pattyn

Hi, here is my 2c on this interesting topic; it is pragmatic rather than academic: - each level needs to catch exceptions, and throw its own exceptions using semantics the caller will understand; - however that would throw away all potentially useful details, as you indicated; hence the original exceptions are added as inner exceptions. - in the end, the top level not only wants to know something failed, it also wants to be able and indicate in what direction a solution might be found. Hence an inner exception "disk full" or "server down" could be very helpful, even when breaking encapsulation. When something goes wrong, I prefer encapsulation gets broken, rather than breaking my head over what may possibly be wrong. :)

Luc Pattyn [Forum Guidelines] [My Articles]

Fixturized forever. :confused:

CodingYoshi

You know what Luc, You make a good point and I think I don't want my head broken either! Thanks for the comments--they are very helpful. I am, although, surprised only you posted a response to such an interesting and open ended question.

Jon Rista

Before I go into a discussion on proper exception management, I need to bring up a fundamental architectural issue. There are a variety of architectural styles, and usually each team or architect will choose the style they like best. However, an extensive amount of research has gone into how "effective" an architecture is in enabling developers, testers, etc. develop a deliverable product. So, I'm going to throw out one of the rules of effective design here: Isolation. Your current design is a very dependant design. Your Person class is tightly coupled in two ways. First off, its tightly coupled to a specific concrete implementation of the PersonManager. Second, its tightly coupled to a specific persistance mechanism. Both couplings are bad, no other way to put it really. If we look at some of the most effective software development methodologies today, DDD and TDD will rise to the top. Both advocate the isolation of classes from each other, and both advocate the use of dependancy injection to improve decoupling (help achieve 'loose coupling'). Your Person entity should be simple, and should not be aware of the persistance object (PersonManager). This means Person can't save itself, so the save operation goes into a person 'service'. You would end up with something like this:

// Service layer
class PersonService
{
Person Load(int id)
{
// Validate ID is greater than 0
PersonManager mgr = new PersonManager();
Person person = mgr.GetByID(id);

    return person;
}

void Save(Person person)
{
    PersonManager mgr = new PersonManager();
    
    if (person.ID <= 0)
    {
        mgr.Insert(person);
    }
    else
    {
        mgr.Update(person);
    }
}

}

// Business Layer
class Person
{
int ID { get; set; }
string Name { get; set; }
}

// Data Access Layer
class PersonManager // This is really a 'Repository' (http://blogs.hibernatingrhinos.com/nhibernate/archive/2008/10/08/the-repository-pattern.aspx)
{
Person GetByID(int id)
{
// Load a person object from the database
}

void Update(Person person)
{
    // Update a person in the database
}

Person Insert(Person person)
{
    // Insert a person into the database, returning the person with an updated Key (ID in this case)
}

void Delete(Person person)
{
    // Delete a person from the database
}

}

With the above,

Leftyfarrell

Building on Jon's answer, we have similar layers and while not necessary, here is a glimpse of what we did. Define classes for DataAccessLayerException and BusinessLayerException and UILayerException. Then we used the MS Patterns and Practices Exception Handling Application Block to help us handle unhandled exceptions (and by handle, I mean log, determine whether or not to rethrow, etc.) at the boundaries. If the DAL catches an exception, it is wrapped in a DataAccessLayerException and rethrown. The Business Layer can then catch the DataAccessLayerException and deal with it. If an Exception is thrown in the BusinessLayer then it is wrapped in a BusinessLayerException and rethrown. The handling of different exception types can be defined within the application block configuration. In our case, the application block still only provides handling for unexpected errors. We would still have a try.. catch to internally handle errors that you can code for and recover from. The application block handles everything else and logs it the listener(s) of our choice (defined by configuration).

Jon Rista

Lefty has some good suggestions, so to clarify with some code examples, here is an updated version of the snippet I posted before. I have made a couple other important changes that will further improve your decoupling and make your product more maintainable in the long run:

// API - This is shared between all layers
interface IRepository
{
T GetByID(int id);
T Insert(T item);
void Update(T item);
void Delete(T item);
}

class DataAccessException: ApplicationException
{
// ...
}

class BusinessException: ApplicationException
{
// ...
}

class PresentationException: ApplicationException
{
// ...
}

// Service layer - Primary exception handling here
class PersonService
{
public PersonService(IRepository repository)
{
_repository = repository; // Inject the repository, so it can be mocked when you unit test PersonService
}

private IRepositoru \_repository;

Person Load(int id)
{
    // Always throw ArgumentExceptions for parameter validations, rather than BusinessExceptions
    if (id <= 0) throw new ArgumentException("The Person ID must be greater than zero."); 

    try
    {
        Person person = \_repository.GetByID(id);
        return person;
    }
    catch (DataAccessException ex)
    {
        throw new BusinessException("An error occurred while accessing data.", ex);
    }
    catch (ArgumentNullException ex)
    {
        throw ex;
    }
    catch (ArgumentException ex)
    {
        throw ex;
    }
    catch (Exception ex)
    {
        throw new BusinessException("An error occurred while processing your request.", ex);
    }
}

void Save(Person person)
{
    if (person == null) throw new ArgumentNullException("person");
    
    try
    {
        if (person.ID <= 0)
        {
            \_repository.Insert(person);
        }
        else
        {
            \_repository.Update(person);
        }
    }
    catch (DataAccessException ex)
    {
        throw new BusinessException("An error occurred while updating data.", ex);
    }
    catch (ArgumentNullException ex)
    {
        throw ex;
    }
    catch (ArgumentException ex)
    {
        throw ex;
    }
    catch (Exception ex)
    {
        throw new Busin

Curtis Schlak

I second John's advice; the Active Record[^] pattern never sat well with me. As an added caveat, be careful that you do not end up with an Anemic Domain Model[^]. I don't usually pass on Fowler's Nibblets of WisdomTM, but he hits the nail on the head with this one. In short, developers will often just define a bunch of classes consisting of a bunch of properties so that they can feel like they're using Object-Oriented techniques. Don't forget that a Person should also have behaviour. (Personally, I've known plenty of misbehaving persons! : )

"we must lose precision to make significant statements about complex systems." -deKorvin on uncertainty

Curtis Schlak

I don't think that anyone addressed your first question, so I'll give it a shot. In OO, encapsulation (or information hiding) concerns the way that we hide the implementation of our class behind a stable design. Thus, if I have the following class

public class Person
{
public DateTime Birthdate { get{ ... something secret ... } }
public int Age { get { ... something secret ... } }
}

then it doesn't matter to the code that uses the Person class if the value for Age gets computed from the Birthdate every time, caches the initial calculation in a private int, or uses some fancy Web service to calculate it. We are hiding the internals of the class implementation which, according to OO, everyone else doesn't care about. Now, in terms of the System.Exception class, it has that property InnerException because, when this is set, the instance of the Exception that you catch was created by another exception which gets stored in that InnerException property. You'll note that the only type information that we have about the InnerException is that it is of type System.Exception, no leakage at all. This does not break encapsulation because, in the context of the creation of the exception that you've caught, another exception was the cause of it and knowing that exception isn't bad. You don't know how the InnerException was set, how the value gets returned to you, or what's going on in the containing exception instance. Now, if catching code has something like the following somewhere

try
{
... exception thrown here ...
}
catch(Exception e)
{
if(null != e.InnerException)
{
if(e.InnerException is SqlException)
{
... do something SqlException specific here ...
}
}
}

then I would argue that you did not handle the SqlException deeply enough in your code or that it should not have been caught until now in a separate catch(SqlException se) block.

"we must lose precision to make significant statements about complex systems." -deKorvin on uncertainty