Creating an abstraction just for exception handling - a pattern or anti-pattern?

Question

Assume that an external library or framework not under our control exposes a Controller API:

abstract class Controller {
 abstract fun call(): Result
}

Assume that we want to handle exceptions from all of our Controller implementations using the same exception handling logic:

abstract class ExceptionHandlingController : Controller {
 final override fun call(): Result {
 try {
 return callInternal()
 } catch (e: Exception) {
 log(e.message)
 // other exception handling stuff
 }
 }
 abstract fun callInternal(): Result
}

My initial concern here is that this does not feel like a necessary abstraction, and may constitute an anti-pattern; specifically:

God Object

(sometimes also called an omniscient or all-knowing object) is an object that references a large number of distinct types, has too many unrelated or uncategorized methods, or some combination of both.

Poltergeist

Useless classes with no real responsibility of their own, often used to just invoke methods in another class or add an unneeded layer of abstraction.

The latter definitely seems to be what is happening here, whilst I'm also concerned that a God object is what it will become if other developers start adding more things they deem "common".

An alternative approach might be to use an extension method:

fun Controller.callWithExceptionHandling(action: () -> Result): Result {
 try {
 return action()
 } catch (e: Exception) {
 log(e.message)
 // other exception handling stuff
 }
}
class MyController : Controller {
 override fun call() = callWithExceptionHandling {
 // return or throw
 }
}

This feels like a far more flexible approach and eliminates the need for a layer of abstraction.

Questions

1. Is the abstraction approach a pattern or anti-pattern, and if so, which?
2. What benefits if any does the abstraction approach have over the extension function approach, or vice-versa?

Answer 1

This example of routing all exceptions to a log does not show an anti-pattern, quite the opposite, it is an example for a well-known GoF pattern - the Template Method pattern.

ExceptionHandlingController, as shown, is not a god object. It may become one, but currently I see only one functionality there, and no references to "a lot of unrelated types".

It is also not a Poltergeist, since it has a clear and useful purpose (piping exceptions into a log in a uniform and DRY manner).

The only Poltergeist I can spot in your example is the abstract class Controller itself, in that (empty) form, it is an unneeded abstraction. I would probably remove that class and rename ExceptionHandlingController to Controller. In case Controller is part of the framework, don't overthink this, this extra layer does not really hurt and Controller is probably not as empty as your question pretends.

To your extension function: when you explicitly want every controller to put exceptions into the log, that solution may be error-prone, every new derived controller has to call the extra function callWithExceptionHandling, and every dev using it must not forget to do so. However, maybe that's what you want - some controllers piping their exceptions into a log, others not. In that case, go ahead.

• Matthew Layton: The Controller class is not under our control, and so cannot be removed or refactored, otherwise I would tend to agree. To your latter point regarding DRY "every dev must not forget to do so" would also apply where a dev would be required to remember to extend ExceptionHandlingController instead of Controller, would it not?
• Doc Brown: @MatthewLayton: that depends on the code which instantiates and uses the controllers. If that code exclusively expects derivations of ExceptionHandlingController, extending the Controller will not create something which works. The extension method approach is fine when you want to allow some controllers with logging, and others without.
• Matthew Layton: The code that instantiates Controller expects any implementation of it, and it's possible that there may be cases where exception handling might not be necessary. The other thing that I actually neglected to mention is that there are two base controller types InboundController and OutboundController, so using abstract classes for exception handling would require two different types of abstract class.
• Doc Brown: @MatthewLayton: hard to give you answer for a moving target. Write a better question, get better answers.

Answer 2

I'd call this out as a code smell. Here's my reasoning :

1. Exceptions are meant to bubble up and be handled. Somewhere in your code there is an entry point which makes the top level call and this can handle the exception.

2. If you are programming within a MVC framework exception handling should already be an option within that framework. You likely have a pipeline you can put handlers in or a default way of dealing with the exceptions, which essentially does the same thing as your code.

If you are making these kind of structures yourself, I would query whether you have missed a simpler, or built in, solution.