views:

69

answers:

5

i have some business logic that traps some logically invalid situations, e.g. trying to reverse a transaction that was already reversed. In this case the correct action is to inform the user:

Transaction already reversed

or

Cannot reverse a reversing transaction

or

You do not have permission to reverse transactions

or

This transaction is on a session that has already been closed

or

This transaction is too old to be reversed

The question is, how do i communicate these exceptional cases back to the calling code, so they can show the user?

Do i create a separate exception for each case:

catch (ETransactionAlreadyReversedException)
    MessageBox.Show('Transaction already reversed')
catch (EReversingAReversingTransactionException)
    MessageBox.Show('Cannot reverse a reversing transaction')
catch (ENoPermissionToReverseTranasctionException)
    MessageBox.Show('You do not have permission to reverse transactions')
catch (ECannotReverseTransactionOnAlredyClosedSessionException)
    MessageBox.Show('This transaction is on a session that has already been closed')
catch (ECannotReverseTooOldTransactionException)
    MessageBox.Show('This transaction is too old to be reversed')

Downside for this is that when there's a new logical case to show the user:

Tranasctions created by NSL cannot be reversed

i don't simply show the user a message, and instead it leaks out as an unhandled excpetion, when really it should be handled with another MessageBox.

The alternative is to create a single exception class:

`EReverseTransactionException`

With the understanding that any exception of this type is a logical check, that should be handled with a message box:

catch (EReverseTransactionException)

But it's still understood that any other exceptions, ones that involve, for example, an memory ECC parity error, continue unhandled.

In other words, i don't convert all errors that can be thrown by the ReverseTransaction() method into EReverseTransactionException, only ones that are logically invalid cause of the user.

+1  A: 

I recommend the single exception with a cause identifier. The cause itself could be an exception which is wrapped in your user exception, although I would consider this mostly for debugging purposes, or as a means to derive additional details.

Your main exception includes an identifier, which identifies succinctly what the user has done wrong. It can be used as the basis for retrieving localized messages to show the user, for linking to user documentation, help, troubleshooting, and other assistance.

The message ID is also useful as an error code which can be used when reporting problems and for documenting solutions in support documentation for your support team.

I use a superclass for all user-level exceptions that allow the use of an ID to identify the situation or cause. All the IDs are documented, and each has at least one test cases to provoke the exception.

mdma
+2  A: 

I find that there are various broad categories of exception:

  1. TransientException - what you just tried didn't work, but if your try again it might. Used for cases such as Database not currently available.
  2. InvalidRequestException - you just asked for something that can't be done (Your examples fit here)
  3. SystemException - The system is sick, we've forgotten everything you just said, your session is dead, you need to start all over again.

I would have these three main types of exception and catch each of them, there being obvious specific actions in each case. All my exceptions derive from these three types.

djna
+1 because i really like you simple descriptions of each case, and especially *"the system is sick"* Aaronaught seems to agree with you, using `ClientException` and `BusinessRuleException`. **Edit:** What's the name of your three base exceptions in each case?
Ian Boyd
+3  A: 

To me the rule of thumb is whether it is useful for the user to see the exact error message. This varies wildly between different types of applications. A desktop app used by millions of "average users" is a very different case from an enterprise web app used by a coupe hundred trained professionals.

For the former, it might be better to display a generic "system error, please restart" type message instead of technical details which the user doesn't understand and is usually not bothered to forward to the support department anyway (unless it can be done with the press of a button).

Our project is of the latter case, and the users typically forward problems to support. So we try to improve the error messages to contain information relevant to the support team. Since ours is a legacy app, we are much less worried about memory parity errors than plain null pointer exceptions and logical errors in the code though.

As for the number of distinct exception types, I am a fan of simplicity so I try to get by with the minimum necessary number of exception types, one for each distinct error category. What constitutes a separate category is also defined by how and when that bug can occur, and how and when it is handled. Since your cases above all are associated with the same use case, I would use a single exception type with specific detail messages.

Péter Török
i used memory ecc parity errors to avoid a potential holy war about how to handle non business-rule error (e.g. what do do when there's a primary key violation, or the TCP/IP connection failed - things that my code technically could try to handle; but i'd rather just not) So by picking an error that everyone agrees i can do *nothing* about, i get cut to the chase and focus on errors that have no solution except "try again"
Ian Boyd
@Ian, I see your point - the example did sound kind of weird to me at first :-)
Péter Török
+1  A: 

Are you deriving all of your exceptions from the base Exception (or EException or whatever the equivalent is in your language?)

I handle this by deriving all business logic errors from a ClientException (the user provided invalid input) or BusinessRuleException (input was valid but would unwittingly violate some important business or domain rule).

Any exception that derives from either of these roots can be caught and displayed to the user. Other exceptions go to the global exception handler, unless the code knows specifically how to handle them.

(Actually, that's not entirely accurate. What really happens is that the global exception handler itself recognizes these exceptions and handles them differently. But the principle is the same.)

Aaronaught
Now i'm torn between this being the marked answer, or djna's.
Ian Boyd
@Ian: Since there is no objectively/technically correct answer here, you should accept whichever one best solves *your* particular problem. Our answers are complementary; mine focuses on the hierarchy of *expected* exceptions while his is a broader categorization of *all* possible exceptions. Whichever one helps you more.
Aaronaught
Well i came back to stackoverflow, to find the answer i remembered, that gave concrete examples of two exceptions, and the logic of when they are thrown. Turns out it was this answer, rather than the one i had previously accepted. On the other hand the accepted answer is a super-set of yours, but lacks the concrete example. They should be co-answers.
Ian Boyd
+1  A: 

You should create separate exceptions when you need (or expect to need) different types of behavior to handle the different exceptions. If you just want different messages displayed, but the basic behavior will be the same for all of them, then you probably just want to derive one exception class from std::runtime_error:

class transaction_error : public std::runtime_error { 
public:
    transaction_error(std::string const &caption) : std::runtime_error(caption) {}
};

which you'd throw something like:

throw transaction_error("Transaction already reversed");

...and catch something like:

try { 
    execute_transaction(transaction_data);
}
catch(transaction_error const &e) { 
    MessageBox(NULL, e->what(), "Transaction Error", MB_OK);
}
Jerry Coffin