views:

285

answers:

7

I have a pattern that repeats for several member functions that looks like this:

int myClass::abstract_one(int sig1)
{
  try {
    return _original->abstract_one(sig1);
  } catch (std::exception& err) {
    handleException(err);
  } catch (...) {
    handleException();
  }
}

bool myClass::abstract_two(int sig2)
{
  try {
    return _original->abstract_two(sig2);
  } catch (std::exception& err) {
    handleException(err);
  } catch (...) {
    handleException();
  }
}

[...]

int myClass::abstract_n(bool sig3a, short sig3b)
{
  try {
    return _original->abstract_n(sig3a, sig3b);
  } catch (std::exception& err) {
    handleException(err);
  } catch (...) {
    handleException();
  }
}

Where abstract one through n are methods of a pure virtual abstract interface for which myClass and _original are concrete implementations.

I don't like that this pattern repeats in the code and would like to find a way to eliminate the repeating try / catch pattern and code as a single abstraction, but I can't think of a good way to do this in C++ without macros. I would think that there is a way with templates to do this better.

Please suggest a clean way to refactor this code to abstract out the repeated pattern.

+1  A: 

I don't have an answer except to suggest that you might be better off avoiding exception handling altogether and relying instead on Smart Pointers and Boost Scope Exit to do all your resource clean up. That way you won't have to catch exceptions unless you can do something about them, which is rarely the case. Then you can do all exception handling in one centralized place higher up the call chain for error reporting and such.

ShaderOp
@ShaderOp in this case the exceptions being thrown are in a 3rd party library that provides a concrete implementation of the abstract interface and thus cannot be avoided.
WilliamKF
+11  A: 

One option, if there are a limited number of function arities, would be to use a function template:

template <typename ReturnT, typename ClassT>
ReturnT call_and_handle(ClassT* obj, ReturnT(ClassT::*func)()) 
{
    try {
        return (obj->*func)();
    }
    catch (const std::exception& ex) {
        handleException(ex);
    }
    catch (...) {
        handleException();
    }
    return ReturnT();
}

This assumes that handleException is some non-member function, but it is easy to modify it if it is a member function. You need to decide what call_and_handle returns if an exception is handled; I have it returning an initialized ReturnT as a placeholder.

This reduces your member functions to:

int myClass::abstract_one()
{
    return call_and_handle(_original, &myClass::abstract_one);
}

You would need a separate function template for calling functions that have one parameter, two parameters, etc.

If you have functions that have an unwieldy number of parameters and you were really desperate, you could use a macro (I wouldn't really recommend this, though):

#define CALL_AND_HANDLE(expr)           \
    try {                               \
        return (expr);                  \
    }                                   \
    catch (const std::exception& ex) {  \
        handleException(ex);            \
    }                                   \
    catch (...) {                       \
        handleException();              \
    }

Which can be used as:

int myClass::abstract_one()
{
    CALL_AND_HANDLE(_original->myClass::abstract_one());
}

As an aside, if you catch (...) and do not rethrow the caught exception, you should in most cases terminate the program.

James McNellis
@James McNellis, yes the exception is being rethrown inside handleException().
WilliamKF
@WilliamKF: In that case, I'd just comment the `return ReturnT()` line with a "this line is unreachable" comment or something along those lines.
James McNellis
@James McNellis so the compiler will give warning or error in such case? In the forthcoming C++0x will there be a way to avoid the return statement to make the compiler happy?
WilliamKF
@James McNellis will the ReturnT() work if ReturnT is void?
WilliamKF
@WilliamKF: Yes, `ReturnT()` will work for any type that is default constructible and for `void`. I'd only include the return at the end to silence warnings about some paths not having a return and because it gives you a good place to document that the exception handler functions will not return.
James McNellis
@James McNellis the handleException() is indeed a member function of myClass.
WilliamKF
@WilliamKF: Then you need to pass `this` into `call_and_handle()` as well so that `handleException()` can be called as a member function.
James McNellis
@James McNellis I thinking the extra template complexity is not warranted here, but thanks for the insight. I have three different signatures across six abstract interfaces, so would need three templates. Seems like a job for C++0x variadic templates.
WilliamKF
Certainly the return is necessary in the case where exceptions are not thrown by the called function?
dash-tom-bang
This is good but it seems it is too abstracted to be very easy for a casual observer to determine what is going on.. this may even be worse than the original problem IMHO
0A0D
@0A0D Yes, without variadic template it is worse, so since I'm not on C++0x, I'm leaving the code unchanged.
WilliamKF
@dash-tom-bang: That is handled in the `try` block itself.
James McNellis
@0A0D: It really depends. I have eliminated thousands of lines of repetitive code like this from legacy projects by using a few extremely generic function templates. Less code is better code. :-)
James McNellis
@James: thanks for fixing my answer :vP … it's very similar to yours anyway
Potatoswatter
@james oh yeah.
dash-tom-bang
@WilliamKF: Do you want variadic? Because there are no ellipsis or use of stdarg.h in this answer.
0A0D
@0A0D No, not ellipsis for my functions, but I thought variadic allowed a template to take variable args, and thus be a single template that is used to call functions with signatures that vary in their number of arguments.
WilliamKF
@WilliamKF: A templated function is not variadic. Variadic means you accept a varied number of arguments (one versus two versus three). This is implemented using the cstdarg. At any rate, you need use ellipsis to have a variadic templated function and I think that is only available in C++0x
0A0D
+7  A: 

I asked a very similar conceptual question, see http://stackoverflow.com/questions/2466131/is-re-throwing-an-exception-legal-in-a-nested-try.

Basically, you can move the various exception handlers to a separate function by catching all exceptions, calling the handler and rethrowing the active exception.

void handle() {
 try {
  throw;
 } catch (std::exception& err) {
   handleException(err);
 } catch (MyException& err) {
   handleMyException(err);
 } catch (...) {
   handleException();
 }
}

try {
   return _original->abstract_two(sig2);
} catch (...) {
   handle();
}

It scales well with more different exception kinds to differenciate. You can pack the first try .. catch(...) into macros if you like to:

BEGIN_CATCH_HANDLER
return _original->abstract_two(sig2);
END_CATCH_HANDLER
Alexander Gessler
+1: I like this idea.
James McNellis
I should add that, after asking this once on SO, I've used it several times and I always found it quite elegant. So I'm wondering whether this pattern has a name?
Alexander Gessler
@Alexander: I've heard it called the "exception dispatcher idiom".
GMan
Thanks! :-----)
Alexander Gessler
Yeah, GMan's right - here's an example of a place that calls it that: http://www.parashift.com/c++-faq-lite/exceptions.html#faq-17.15
SCFrench
+1  A: 

Use boost::function and boost::bind. Works with any function signature so long as the return type matches;

#include <boost/function.hpp>
#include <boost/bind.hpp>

using boost::function;
using boost::bind;

template<typename T>
T exception_wrapper(boost::function<T()> func)
{
  try {
    return func();
  } catch (std::exception& err) {
    handleException(err);
  } catch (...) {
    handleException();
  }
}

// ways to call
int result = exception_wrapper<int>(bind(libraryFunc, firstParam));
// or a member function
LibraryClass* object;
result = exception_wrapper<int>(bind(&LibraryClass::Function, object, firstParam));

// So your wrapping class:
class YourWrapper : public SomeInterface
{
public:
    int abstract_one(int sig1)
    {
        return exception_wrapper<int>(bind(&LibraryClass::concrete_one, m_pObject, sig1));
    }

    bool abstract_two(int sig1, int sig2)
    {
        return exception_wrapper<bool>(bind(&LibraryClass::concrete_two, m_pObject, sig1, sig2));
    }

    // ...

private:
   LibraryClass* m_pObject;
};
Nicholas M T Elliott
+1  A: 

As a variant on Alexander Gessler's solution you can omit some of the braces that make this implementation a little long. It does exactly the same thing, just with a little less { } verbage.

void handle() try 
{ 
  throw;
}
  catch (std::exception& err) 
{
  handleException(err);
}
  catch (MyException& err)
{
  handleMyException(err);
}
  catch (...)
{
  handleException();
}



int myClass::abstract_one(int sig1) try 
{
  return _original->abstract_one(sig1);
}
  catch (...)
{
  handle();
  return -1;
}
Michael Anderson
@Michael Anderson Interesting, I've never seen making try the outer most expression of a function body, this is standard syntax?
WilliamKF
@WilliamKF Its called a "function try block" and is covered lightly in the 15.1.4 section of my copy of the draft standard [N1905=05-0165] from 2005.
Michael Anderson
@Michael Anderson So what does the compiler do if you exit the catch block without throwing, what is the return value?
WilliamKF
From memory the catch block can return something (and in my example should - so I've updated it), or the handle() must rethrow something .
Michael Anderson
+1  A: 

My answer is conceptually similar to James McNellis', except that I use boost::bind to do the heavy lifting:

using boost::bind;

class myClass
{
public:
  myClass(origClass * orig) : orig_(orig) {}

  int f1(bool b) { return wrapper(bind(&origClass::f1, orig_, b)); }
  bool f2(int i) { return wrapper(bind(&origClass::f2, orig_, i)); }
  void f3(int i) { return wrapper(bind(&origClass::f3, orig_, i)); }
private:
  origClass * orig_;

  template <typename T> 
  typename T::result_type wrapper(T func)
  {
    try {
      return func();
    } 
    catch (std::exception const &e) {
      handleError(e);
    }
    catch (...) {
      handleError();
    }
  }
};

Note that I wouldn't use a boost::function here as it can interfere with inlining.

SCFrench
+1  A: 

My answer is: do nothing at all. The first code example as shown is fine. So what is there's repetition? It is plain and clear, and does what it looks like it does. It can be understood with no extra mental burdens past the code seen and general knowledge of C++.

Consider your motive for asking this question.

Where I'm coming from is past projects where code had to be examined by others - not PhD Comp Sci experts, but federal inspectors, mechanical engineers, self-taught programmers, scientists, etc. Smart people, all of them (or most of them), but only an off-the-deep-end chrome dome PhD, or a younger programmer out to impress everyone with their hi IQ, would appreciate the clever "solutions" to this question. For the places I've been, nothing beats plain clear code that does what it says, with no mental burden of having to keep in mind the meaning of dozens of classes, macros, etc. and recognizing "design pattern" understood properly only be experienced software engineers.

Increasingly, I find C++ (and Java and C#) code becoming more sophisticated in a coding sense, but needing to be understood by non-experts in C++.

Of course YMMV, depending on the intended audience and source of potential future maintenence programmers. Sometimes clever coding is necessary to achieve certain goals. Is your case an example?

DarenW
I feel the accepted solution is a reasonable balance between obfuscation and reduced code to maintain and understand.
WilliamKF