Question

Our team has different opinions about the practice of using self-defined exception for handling the business logic.

Someone says it is good as the exception comes meaningful and readable, and the opposition thinks it is redundant to define every possible exception and catching exceptions is performance hurting.

Would you share your thoughts and experience?

Was it helpful?

Solution

Like many things in programming, using custom exceptions is good if done for the right reasons and in the right situations.

If a built-in exception can acurrately describe the situation at hand, use it. E.g. FileNotFoundException, TimeoutException, KeyNotFoundException, etc.

If non of the built-in exception classes describe the situation, then you should make your own. If custom exceptions were bad, the built-in classes would be sealed or final or similar. However, don't go nuts. Just like with classes, a given exception type should probably be thrown from more than one place, otherwise it is likely unnecessarily specific.

However, a better rule of the thumb for whether to have custom exceptions is to consider what the caller will do about it when the exception gets thrown. If you have three erroneous situations that are handled in three very different ways, it makes sense to have three types because then the caller can have three catch blocks accordingly. The alternative, which is to inspect the message or data to decide what to do, is more complicated and thus more prone to error.

If all three situations instead are handled identically, it may not be worth the hassle of additional types, when just using the exception message should be enough to diagnose what the situation is. I've had to deal with many situations where a function can throw a dozen different exception types, but that to me as the caller, have inconsequential differences, so laziness kicks in and I just catch {} without the exception type and feel a little guilty about it. Don't cause that to happen.

As for one of the claims you wrote:

catching exceptions is performance hurting.

While that is true, there is virtually no difference between having custom exceptions that are caught and thrown vs just using ApplicationException for everything and making decisions based on the exception message. However, the for former situation, it is considerably easier for us humans to reason about what is happening during program execution.

OTHER TIPS

There's one thing I see in the existing answers that bothers me:

Catching custom exceptions might be code smell.

If you are creating custom exceptions, you shouldn't be catching them. Exceptions are for when things go horribly wrong and recovery is impossible. Exceptions that can be handled are either your own fault or vexing.

Taking from the MSDN article Vexing Exceptions:

I first classify every exception I might catch into one of four buckets which I label fatal, boneheaded, vexing and exogenous.

Fatal exceptions are not your fault, you cannot prevent them, and you cannot sensibly clean up from them.

Boneheaded exceptions are your own darn fault, you could have prevented them and therefore they are bugs in your code.

Vexing exceptions are the result of unfortunate design decisions.

And finally, exogenous exceptions appear to be somewhat like vexing exceptions except that they are not the result of unfortunate design choices. Rather, they are the result of untidy external realities impinging upon your beautiful, crisp program logic.

I recommend reading the whole post.

Any design that creates vexing exceptions is a bad design. You (and anyone else writing code against your api) has to catch them, and doing so is vexing: so avoid creating new vexing exceptions.

Custom exception types should either be exogenous ("I'm sorry, Dave, but the database went offline")--and most of these kinds of exceptions have already been written--or Fatal and you should never handle fatal exceptions.

This last point is not adequately presented in the other answers. A bad credit card number is a validation error not an exception. Errors are communicated to the user because they can fix it. Exceptions are things the user can never do anything about. Exogenous exceptions are the exception here, as they are the fault of the world at large and the user needs to be aware of them, even if they can't fix it (but sometimes they can: Internet is disabled, the disk was ejected, the information was wrong. Regardless, there is nothing you as the developer can do to fix it: catch, display, don't crash).

A FamilyNameException is boneheaded: your code had a bug in it. Do not create boneheaded exceptions, do not handle them, write your code correctly in the first place so that these do not occur. Custom exception types are inappropriate for this reason.

A LoginException is either exogenous (the database is offline) or PEBKAC. If it is a PEBKAC error, it isn't an exception. If it is exogenous, why is the existing exception type not sufficient? There may be cases where custom exception types are relevant, but be sure to communicate properly! For example I wrote some code that communicated with an external service, which took time. My coworker was trying to ask my code for the results before the request had time to return, so my code had to generate an exception. However, he took this exception (which was boneheaded in nature) and treated it like a vexing: caught the error and assumed all zeros, thereby introducing a bug: the data retrieved from the external service was wrong! (No, the data was right, but he wasn't displaying the retrieved data and blamed my code). I ended up having to fix his code twice (he removed my first fix about 2 weeks after I added it).

To sum up:

  • Catching any fatal or boneheaded exception is code smell.
  • A custom vexing exception class is code smell. See also: How to avoid vexing exceptions.
  • Be careful with custom exogenous exceptions: they are either redundant or can be mistaken for vexing exceptions.

The issue I see when there's a lot of custom exceptions is that they're not used what they're meant for: catching exceptional behaviour. I see them used to catch everything that doesn't follow the happy path. Then you're using exceptions for expected behavior. For flow control. Think by yourself: an uncaught exception basically tells: "I can't deal with this, what's happening now should crash the process".

Is it really unexpected that someone types digit wrongly in their creditcard-number? And should it crash your application? I've seen exceptions for that and in some contexts they're even valid. But in most contexts they're not and you should make a path for it to deal with that situation without using exceptions.

My favorite use of inheritance is to give exceptions meaningful names.

class LoginException extends RuntimeException {}

That's it, one line.

It's perfectly ok to do this even if you don't catch it specifically or at all. You can do this for readability.

Anyone that argues against this for efficiency reasons needs a real good argument or needs to stop throwing exceptions so often.

Exceptions are expensive to construct regardless of whether or not you define your own. That's why exceptions should be exceptional.

Since the performance difference is trivial, this is a style choice. One that is best informed by readability considerations. With good descriptive names, this can be a godsend to a maintenance developer. Non-descriptive names, created for no better reason than "we always make our own exceptions", are painful clutter. Please don't use this to do that.

Someone says it is good as the exception comes meaningful and readable, and the opposition thinks it is redundant to define every possible exception and catching exceptions is performance hurting.

These are not opposing views, as they are both right, to a degree.

Writing defensive code, with lots of checks and exceptions, complicates your code and so can make it more difficult to read and maintain. It also might have an impact on performance. Though in the latter case, care should be taken that you are not indulging in premature and/or micro-optimisation by worrying about this. It's extremely likely that the performance hit is negligible and unless you have profiled the code, you have no way of knowing if it's having a significant performance boost.

By creating your own exceptions, Having a function throw a FamilyNameException rather than a NullReferenceException may help with debugging that function as it's clearer what has gone wrong. But be pragmatic. If you have FamilyNameNullException, FamilyNameEmptyException, FamilyNameContainsOnlySpacesException etc, you may be providing a bit more clarity as to what's wrong, but it comes at the cost of increasing the size of the code base, which again will hurt maintainability as the more code, often the harder it is to learn and work with.

So compromise: avoid too much exception throwing and avoid too many custom exceptions.

I'm of the opinion (and admittedly this is an opinionated answer that's based on the sort of particular needs I encounter in my domain, the tools and languages I use, etc, and I wouldn't be surprised to find many disagreeing), that we should throw and catch "as generally as possible". So I guess I'm in the camp that wants to minimize the number of exception types there are to throw and especially minimize the number to uniquely catch (though not for performance reasons; I don't see any practical concerns there).

Now I don't expect everyone to agree with this so I'll just focus on why I think this way given my particular needs and experiences and tools involved.

And "generally as possible" does not necessarily mean using one exception type/class for every possible exceptional runtime situation the code can encounter. There might be cases where we need more information than that in a catch block which might practically call for defining a new exception type and some cases where we might have two or three catch blocks for a corresponding try.

Example: LocalizedException

For example, in my language (C++), std::exception only contains a string method (called what) to return a string describing the error. In some of the specific types of runtime exceptions encountered specifically in my particular software, it might be useful to define a LocalizedException class. If we catch that type of exception of subtypes of it, perhaps the what method is not overridden to return a string describing the error, but actually a string describing a key to use into our application-specific message table which associates that key to a descriptive string translated to the user's chosen language. That's still somewhat of a contrived example I tried to come up with since it's likely better to just override what for that type to do the lookup on the fly instead of making it the catcher's responsibility.

But even with this contrived example, you can see that's still rather generalized although a tad less general than std::exception. I'm defining a new type specifically for the purpose of communicating something just a tad more specific than std::exception which still unifies all the possible exceptions that could be thrown specifically by code under our control, with catchers of that particular LocalizedException being expected to do something a bit different with the information provided by it than the most generalized language exception.

C++ and Our Use of Exceptions

In our case the language choice might matter a bit. We don't use exceptions to detect programmer bugs whatsoever, like an array being accessed out of bounds. I can see legit cases in software to want to gracefully recover even in the presence of programmer mistakes, but such runtime checks are not necessarily practical when we're dealing with things as performance-critical as raytracers. So we reserve such checks to try to detect programmer mistakes through asserts which are eliminated away in release builds with the goal of detecting those in our automated tests before we ship.

Our use of exceptions is strictly limited to external runtime errors which are not programmer mistakes, like the user's file being corrupt, with the recovery goal being to tell the user what went wrong that's outside the developer's control and recover gracefully so that they can keep using the application.

Another aspect of C++ that I think tends to change the kind of exception-handling you commonly see in it from some other languages is destructors. I've often seen examples in other languages where, say, someone locks a mutex in a try block with the sole goal is making sure it gets manually unlocked if any exception is thrown, so that might naturally call for code with a whole lot more try and catch blocks all over the place and possibly a greater need to catch more specific types of exceptions. With C++ the recommended way to tackle that problem is just make a scoped mutex which automatically unlocks itself immediately when it goes out of scope (regardless of whether it goes out of scope in an exceptional path or not).

User-End Needs

Probably the reason I favor this is because our user-end needs tend to be simple with respect to handling exceptions. Often it's a matter of the user requesting to execute a command like loading a specific file they choose from a file dialog.

In that case the command's job is to either execute successfully or display an error message to the user and roll back the application state (if it attempts any state changes -- these days I tend to refrain from making commands change application state and instead return new application state absent side effects to avoid the need to roll back transactions in exceptional paths). The error message is often sufficient just being like this:

Failed to load file: "some_user_file.dat". Error: {some more specific reason here}.

For that more specific part of the error message, we do invest effort into trying to make it informative and user-friendly when it's something we can anticipate under our immediate control (i.e., we're the ones that throw the exception, not some third party). For example, we might be parsing a proprietary scene file format which is text-based and sometimes edited or generated by hand by power users. In that case it might be really helpful to them to display where the data is malformed, in which line of text in that original file, etc. So in that case we're the ones parsing and the ones throwing the parsing error, so we try to make it as informative as we can within reason, and might throw the analogical LocalizedException above to translate the resulting message to the user's chosen language.

However, there are like a gazillion runtime exceptions that could thrown outside of our control, including things impossible for us to anticipate in advance, and that'll get to my final rationale for why I favor things this way.

Exceptions Can Be "Implementation Details"

In our cases, what exceptions a function can throw are sometimes "implementation details" of a function, as in something you generally shouldn't need to know about (or even be much concerned about) to use the function correctly.

Of course with well-documented third party libraries and APIs, they do often document what exceptions/error codes their functions can throw/return, and in those cases we do try to anticipate some of the more common ones users can encounter (ex: failing to connect to a server in the underlying socket API) and catch and rethrow the exception into an exception that contains a more information message for users (and possibly with more contextual information). But that doesn't necessarily require that we define a SocketException type. Since our catcher's general purpose is to just display messages of what went wrong to users, it might suffice to just use the analogical LocalizedException in those cases.

But there are polymorphic functions in our case where this is impossible to do, because a lot of our architecture is plugin-based with third parties (including third parties from the future) writing plugins which define and implement these functions. And who knows what sort of runtime errors they will encounter, and report back to our system? We're not mind readers, we don't have time machines. I don't know what sort of errors/exceptions by someone outside of our team writing a plugin 2 years from now in the future could report back to us. So we can't help but, in those cases, just take the associated error and message and just display it to the user with some uber generalized exception type we might throw when encountering an error status in the plugin.

So in our case it's rather often that what can be thrown is an "implementation detail" that is impossible to x-ray, because we don't know why, how, or even when that function will be implemented and by whom. And maybe for those reasons because it's rather common in our case for it to be impossible to anticipate what might be thrown in advance and combined with needs to ship that I've favored the idea of throwing and catching as generally as possible.

Anyway, that's just my whole take for this, and I don't expect everyone to agree since our needs and tools and so forth are all different from one another.

There are two reasons to think about type and content of an exception object:

  1. When the exception is finally logged, what will the log message took like?

  2. Will some caller want to react differently on different kinds of exceptions, e.g. with different retry / recovery strategies?

While the former might just seem a matter of cosmetics, it sould be taken seriously. Having misleading information in a logfile when you hunt for a problem in production, is highly annoying.

From my experience, the second aspect hardly ever matters. I haven't often seen code that really implements different recovery strategies based on exception types. And at least half of these rare cases were misguided (using exceptions as a vehicle of flow control).

As far as both aspects matter to your situation, use either existing exception types or invent your own ones. There's no performance difference between these exception types. Just use the ones that best describe the exceptional situation.

Licensed under: CC-BY-SA with attribution
scroll top