I have three or more different custom exceptions that a class can throw and I need to use try/catch in order to discover which exception was thrown.
In my point of view this piece of code violates the Open/Closed principle, because if we look the definition we see:
“software entities (classes, modules, functions, etc.) should be open
for extension, but closed for modification”
try {
// something that throws
} catch (const FooException& e) {
// ...
} catch (const BarException& e) {
// ...
} catch (const BizException& e) {
// ...
} catch (...)
{
// ...
}
What if I need to catch another exception? I will need to insert another catch statement, right?
Is there another way for doing this? Or the language is somehow forcing me to violate the principle?
10
That depends on how you plan to recover from these exception, meaning what code is executed in the catch statements.
If it’s the same for all 3 exceptions, you could create a superclass for them and catch the superclass. If your method has to throw a new exception, just make it extend the superclass and it would still be caught by the caller and would not need any changes in the client code.
Generally tough, if a method throws that many exceptions it’s usually a hint that the method does to much and you might want to consider refactoring it.
If you want a more detailed answer, it would be helpfull to see a complete code listing of your method.
1
If you just want to log each exception – in some way depending on its specific type, you can split that concern out into a dedicated exception logger.
First, you need a base type – let’s assume you just subclass std::exception
. Now, your main code can just be:
try {
// something that throws
} catch (const std::exception& e) {
ExceptionLogger::log(e);
throw;
}
Have the log
method use dynamic_cast
to handle all the specific subtypes it knows, and you have just one place to modify when a new exception type is added.
NB. I made log
a static method here, but you could reasonably have a local (or data member, or …) ExceptionLogger
instance configured differently per component, connected to a different output/target logger, etc. etc.
There are essentially two implementation options for the log
method itself:
-
hand-craft something that knows each derived type, and what to do with it. This can use
dynamic_cast
explicitly:void ExceptionLogger::log(std::exception const &ex) { if ((FooException const *e = dynamic_cast<FooException const *>(&ex))) { // handle foo } else if ((BarException const *e = dynamic_cast<BarException const *>(&ex))) { // handle bar } // else ... etc. }
(as you probably noticed, this is actually uglier than the original code, so all we’ve done is de-couple it from the code you didn’t want to modify), or you can move exactly the same try/catch arrangement out of your function – just re-throw in the try block.
-
Write a generic typeid-based dispatcher, so you can de-couple the per-exception-type code from both your original function and the dispatcher. This is a form of ad-hoc visitation (you’re essentially writing a
visit
method for each interesting subclass ofstd::exception
). Now, you can register each new handler externally, without changing either your original code or theExceptionLogger
itself.
3
If you want an OCP-conforming way of handling an unspecified list of exceptions, then you can consider “chain of responsibility”. But it’s not clear from your question whether your code is specified to handle “an arbitrary set of exceptions, each in a different way, all of which will be specified in future and is subject to change”, or whether the fixed list of three that it currently handles is a fundamental and reliable part of the design.
Adding another exception to the list of things that your code needs to handle isn’t “extending” the code you have, it is “modifying” it. So if you’re in the former, flexible, case then you have an OCP violation on your hands because you’ll need to modify your code to handle an anticipated change. If you’re in the latter case, perhaps because Foo
, Bar
and Biz
represent the only three legal base classes for all exceptions that your code is permitted to handle different from the ...
case, then you might be OK.
Provided that “something that throws” has a well-defined and stable interface, then you don’t ever need to add to the list of exceptions you handle. So your code isn’t in itself a violation of OCP. Part of OCP is that you don’t make backward-incompatible changes to existing interfaces that people rely on throughout the code. Throwing a new exception is a backward-incompatible interface change, it is modification.
If “something that throws” has a badly documented interface, or one subject to change in future, such that it can suddenly start throwing something it didn’t throw before, then you do have a violation of OCP: “something” violates it by being open to modification. These violations tend to spread, so your code will have to violate it too.
If “something” is well-specified, but your code is using information about “something” that goes beyond its interface, then you have a problem with over-coupling between components. I’m unsure whether to consider that also an OCP violation, I’m not really in a position where the taxonomy of issues beyond the first is important to me 😉
Note that in any case a complete commitment to OCP is a reasonably high standard. You might find that “I’m not going to write the code this way because it violates the Open/Closed Principle” simply doesn’t suit you or your colleagues. Which is not necessarily to say you should be ignorant of whether you’re violating it or not.
I think Phil’s answer should get accepted in the end because it contains the best info. But to just answer your Is there another way for doing this?
question specifically: here is another way which seperates the exception handling part pretty well, though it depends on the exact situation how usefull it is
template< class Fun >
void TryCatch( Fun f )
{
try {
f()
} catch (const FooException& e) {
// ...
} catch (const BarException& e) {
// ...
} catch (const BizException& e) {
// ...
} catch (...)
{
// ...
}
}
void DoSomething()
{
TryCatch( []()
{
//do something
} );
}