Some questions about handling exceptions and when to throw them

[Steve]

I have read a couple articles online, read my Jesse Liberty book but I am
still confused as to just what the best practices are for using exceptions.
I keep changing how I'm working with them and it has now, after 15k lines of
code resulted in a royal mess!

It's my hope to ask some specific questions with scenario examples and that
some of you might offer a little guidance or general suggestions.

1) string GetCustomerFirmwarePath(string serialNumber) - This is a method
in a class library I have written and it is called from my UI "Presenter"
class' (In case you aren't familiar, Model View Presenter is very similar
to Model View Controller pattern). In the event that a customer can't be
located from the serialNumber parameter, is it a better choice to return
null and check for it or to throw an exception from
GetCustomerFirmwarePath()? Possibly even my own exception, IE:
"CustomerNotFoundException"? I have read that exceptions shouldn't be used
for logic, but I have been spending a lot of time with the Patterns and
Practices group's Composite Application Block and in one of the examples,
they use exceptions in this manner.

2) methodA calls MethodB which calls MethodC, MethodD and MethodE. IE:
PrinterInfo GetEmployeeDefaultPrinter(string employeeFullName)
calls EmployeeInfo GetEmployeeInfo(employeeFullName)
calls ComputerInfo GetEmployeeComputerInfo(EmployeeInfo info)
returns ComputerInfo.DefaultPrinter

So that is a massively stupid design, but I couldn't think of an example
that is easy to visualize (mine is very confusing and wouldn't be a good
example) Point is, if the call to GetEmployeeComputerInfo fails, there is
no clean way to return that error back from GetEmployeeDefaultPrinter() or
if the call to GetEmployeeInfo() fails, same thing. I can return null all
the back down the stack, but short of looking at the log (assuming I'm
logging) there is no clean way to determine where the problem was. This
example makes me think that throwing an "ComputerInfoNotFoundException"
might be a good idea, then I could could catch it in GetEmployeeInfo and
throw a "EmployeeInfoNotFound" exception with the
"ComputerInfoNotFoundException" as the inner exception. Does that makes
sense? Is it a good design? Seems like a lot of exceptions! But what
other clean way could you handle this? Please don't suggest anything like
overriding GetEmployeeComputerInfo(string employeeFullName) and avoiding the
whole call stack, it's just an example, my actual situation has a good
reason for breaking things apart like this.

3) How do you decide what exceptions to catch and which ones not to? I
have read that if I know I can handle it(IE: FileNotFound) then I can catch
it and act accordingly, but that would suggest that it's OK to let all
others bubble up... bubble up to where? Application.Run()? What guidelines
are there?

4) One last one. As I think I mentioned earlier, I'm using the Composite
Application Block. Using the CAB encourages breaking logic into separate
assemblies to be shared by other assemblies for code reuse, etc. There is a
concept of a "Service" which is a basically a singleton that will perform
tasks related to a Workitem. For example, I have a service called
"FirmwareService" and it is responsible for adding new versions of firmware
for our products to the database, retrieving older version, assigning a
particular version to specific customers, etc, etc. An example method is:
string GetCustomerFirmwarePath(Customer customer);
This function is similiar to GetEmployeeDefaultPrinter() in that it makes a
lot of other calls that could result in an error, either acceptable or
fatal. What I have tried to do in my design is avoid having code like this
scattered throughout my Presenters and other task code:
<code>
// Presenter method to get a customer's firmware path and display in the
view
// All manufactured products have a record of what customer they are for
and from that, what firmware should be loaded
ProductInfo productInfo =
_productionService.GetProductRecordFromSerialNum(serialNumber);
if(productInfo == null)
{
// show error
return false;
}

Customer customer = _firmwareService.GetCustomerFromProductInfo(productInfo
;
if(customer == null)
{
// show an error
return false;
}

string firmwarePath = _firmwareService.GetCustomerFirmwarePath(customer);
if(firmwarePath == null || firmwarePath.Length < 1)
{
// show an error
return false;
}
</code>

(that was not real code, so if there are errors, please excuse me)

What I DO want to have is something like this:
<code>
string firmwarePath =
_firmwareService.GetFirmwarePathFromSerialNum(serialNum);
if(firmwarePath == null || firmwarePath.Length < 1)
{
// show an error
return false;
}
</code>

This question is ending up to be a duplicate of #2, but I guess that is my
main question. Given the example above assuming that
GetFirmwarePathFromSerialNum() performs all the tasks in the example code
before it, how do gracefully return errors and handle the exceptions? For
example, if the Customer record was found but not the firmware path, I might
want to do something like allow the user to add firmware for the customer or
something... I dunno, I've written too much, I'll stop.

Hopefully someone will take notice of this very long email and respond with
some insights for me :0)

Thanks for reading,
Steve

 

[V]

Steve,

I will attempt to answer your questions based on what I get implemented
in our projects at work:

1. NO, you should not be using exceptions for Logic in your code. As
for the Application Block, those are more like an extension to the
framework, and are meant to be used by loads of developers. Its like
the framework throwing exception on illeagal use of a class. Not
finding a customer is by no means an illegal use of a class.
Additionally, this code is your own and for your own consumption, so
you should not be using exceptions to drive application logic.

2. For a scenario like this, you can always examine the stack trace
property of an exception to determine where the exception actually
happened. Another way for this is to build up a system of status codes,
and return status codes instead of exceptions to pass information up a
stack.

3. The way I tell my developers is: Catch any exception that you can
imagine occuring and can therefore handle. By handle, I mean that the
exception can be caught and an alternative action can be taken to
address the situation. This could simply even mean building up correct
error messages, or it could mean triggering some other program logic.
As a rule, a global exception handler should be present which catches
all exceptions (which you may not be handling), and handles them, and
logs them, etc.

4. Once again, build a status code framework for usage across your app.

- Vaibhav

 

[Steve]

Hi Vaibhav,

Thank you very much for taking the time to respond, I will review your
answers a couple more times and think about ways I can adapt some of your
suggestions.

I never thought of status codes and when I do think of them it reminds me of
older libraries and APIs I have used, they just don't feel "Latest and
greatest", but I know that doesn't mean it's not a good solution. I think I
will try implementing them in a single library and go form there.

Thanks again,
Steve

 

[Bruce Wood]

1) string GetCustomerFirmwarePath(string serialNumber) - <snip>
In the event that a customer can't be
located from the serialNumber parameter, is it a better choice to return
null and check for it or to throw an exception from
GetCustomerFirmwarePath()? Possibly even my own exception, IE:
"CustomerNotFoundException"? I have read that exceptions shouldn't be used
for logic, but I have been spending a lot of time with the Patterns and
Practices group's Composite Application Block and in one of the examples,
they use exceptions in this manner.

This very much depends upon context, and so looking at the method in
isolation, there's no right answer. Since you have the luxury of both
writing the method and writing its caller, you may be able to make a
definitive choice.

The way to decide is to ask yourself whether not finding a customer for
that serial number is something that should "never happen", or
something that is a common possibility. If it's the former, then an
exception is an appropriate choice; if it's the latter then you should
do one of two things:

1. Return null, but this works only if there's only one error
condition, or there are multiple error conditions but you don't need to
distinguish between them, so there's only one error condition at the
level of GetCustomerFirmwarePath, conceptually speaking, or

2. Provide another method that will check for the error condition, and
then make GetCustomerFirmwarePath fail with an exception. So, you might
right CustomerHasFirmwarePath, which returns true or false, and then
GetCustomerFirmwarePath will die with an exception if there is no
customer, because the caller should have checked first. See the
Framework's TryParse / Parse methods to see this in action.

2) methodA calls MethodB which calls MethodC, MethodD and MethodE. IE:
PrinterInfo GetEmployeeDefaultPrinter(string employeeFullName)
calls EmployeeInfo GetEmployeeInfo(employeeFullName)
calls ComputerInfo GetEmployeeComputerInfo(EmployeeInfo info)
returns ComputerInfo.DefaultPrinter

<snip>
Point is, if the call to GetEmployeeComputerInfo fails, there is
no clean way to return that error back from GetEmployeeDefaultPrinter() or
if the call to GetEmployeeInfo() fails, same thing. I can return null all
the back down the stack, but short of looking at the log (assuming I'm
logging) there is no clean way to determine where the problem was.

This is an exercise in your ability to "see" methods from the outside
rather than from the inside. In effect, you apply the logic from point
#1 over and over again at each level of the call hierarchy. Using your
example:

If GetEmployeeInfo fails (there is no employee with that name), what
does that mean from the perspective of the code that calls
GetEmployheeDefaultPrinter? Is it a normal situation? Or perhaps it's
fatal because the caller should have validated the employee name first?
If it's a normal situation (the caller isn't expected to validate the
employee name) then does the caller need to distinguish between the
employee name being invalid and the employee having no printer? If so,
why wouldn't we force the caller to check the employee name first,
given that it cares about why the employee name doesn't correspond to a
printer? In other words, what's the difference between this:

EmployeeInfo ei = GetEmployeeInfo(employeeFullName);
if (ei != null)
{
PrinterInfo pi = ei.DefaultPrinter;
}

and this:

try
{
pi = GetEmployeeDefaultPrinter(employeeFullName);
}
catch (InvalidEmployeeNameException ien)
{
... handle error ...
}

They're the same amount of code, but the first one doesn't require
exceptions.

In general, if your caller has to distinguish between different error
cases, then it might as well be doing the checking up front, anyway.

One exception might be if your caller has to display error messages to
the user. In these cases I usually do this:

PrinterInfo GetEmployeeDefaultPrinter(string employeeFullName, out
string message);

If GetEmployeeDefaultPrinter returns null then "message" contains an
error message to display to the user. If you care about the reason for
the error in some cases but not in others, you can always provide an
overload:

PrinterInfo GetEmployeeDefaultPrinter(string employeeFullName)
{
string message;
return GetEmployeeDefaultPrinter(employeeFullName, out message);
}

3) How do you decide what exceptions to catch and which ones not to?

I can't remember all of the lists I've posted on this topic, but here
are some ideas. Catch an exception if:

1. You can take some logical, alternative action: If the user has no
default printer then you can abort the print job. Note that this is
different from "machine is out of memory" in which case you want to
crash and burn.

2. You want to transform the exception into a different exception. For
example, you may call a method that throws a DivideByZeroException, but
that might be meaningless to your caller, to whom you might want to
throw an ArgumentException because the DivideByZeroException happened
because the froobitz value was too large.

3. You might want to remember the exception and throw it (or a
different exception) later. For example, if you're processing 1000
items, you might want to remember that there were exceptions and then
throw one exception at the end, rather than bailing at the first error.

4. You want to ignore it, which I suppose is taking logical alternative
action.

I have read that if I know I can handle it(IE: FileNotFound) then I can catch
it and act accordingly, but that would suggest that it's OK to let all
others bubble up... bubble up to where? Application.Run()? What guidelines
are there?

Take a look at the events AppDomain.UnhandledException and
Application.ThreadException.

4) <snip> An example method is:
string GetCustomerFirmwarePath(Customer customer);
This function is similiar to GetEmployeeDefaultPrinter() in that it makes a
lot of other calls that could result in an error, either acceptable or
fatal. What I have tried to do in my design is avoid having code like this
scattered throughout my Presenters and other task code:
<code>
// Presenter method to get a customer's firmware path and display in the
view
// All manufactured products have a record of what customer they are for
and from that, what firmware should be loaded
ProductInfo productInfo =
_productionService.GetProductRecordFromSerialNum(serialNumber);
if(productInfo == null)
{
// show error
return false;
}

Customer customer = _firmwareService.GetCustomerFromProductInfo(productInfo
;
if(customer == null)
{
// show an error
return false;
}

string firmwarePath = _firmwareService.GetCustomerFirmwarePath(customer);
if(firmwarePath == null || firmwarePath.Length < 1)
{
// show an error
return false;
}
</code>

The first question you have to ask yourself is whether in each of those
cases the user should see a different error message. If so, and there's
no way to show a generic error (that meets requirements), then I would
go with the "out string message" solution I mentioned above.

Or, the other solution is to force the caller to check things
beforehand, and have the called method specifically state in its header
comments that passing invalid information results in an exception.
(I.e. it's fatal, because the caller should have checked the arguments
before making the call.) This isn't as bad as it sounds, because the
caller cares about the different error cases anyway, and has to check
either before or after, anyway.

Often, however, the answer is that "The user doesn't care why it
failed, only that it failed," in which case it's enough to simply
return "false" and report a generic failure.

 

[sklett]

Excellent reply, Bruce, thank you for taking the time!
I've got V's and yours printed out. These are exactly the kind of responses
I was hoping for.

"The way to decide is to ask yourself whether not finding a customer for

that serial number is something that should "never happen", or
something that is a common possibility. "

This part is great advice. There are operations in my application that
should never have null values. A situation like "If you are here in this
function, then the customer BETTER exists, otherwise there is another
problem somewhere else"

So in those cases throwing an exception would be a good decision.

OK, thanks again for the response!