Exception handling aware of execution flow

Question

Edit:

For personn interested in a cleaner way to implemenent that, have a look to that answer.

---

In my job I often need to use third-made API to access remote system. For instance to create a request and send it to the remote system:

   #include "external_lib.h"
   void SendRequest(UserRequest user_request)
   {
       try
       {
           external_lib::Request my_request;
           my_request.SetPrice(user_request.price);
           my_request.SetVolume(user_request.quantity);
           my_request.SetVisibleVolume(user_request.quantity);
           my_request.SetReference(user_request.instrument);
           my_request.SetUserID(user_request.user_name);
           my_request.SetUserPassword(user_request.user_name);
           // Meny other member affectations ...
       }
       catch(external_lib::out_of_range_error& e)
       {
           // Price , volume ????
       }
       catch(external_lib::error_t& e)
       {
           // Here I need to tell the user what was going wrong
       }
   }

Each lib's setter do checks the values that the end user has provided, and may thow an exception when the user does not comply with remote system needs. For instance a specific user may be disallowed to send a too big volume. That's an example, and actually many times users tries does not comply: no long valid instrument, the prices is out of the limit, etc, etc. Conseqently, our end user need an explicit error message to tell him what to modify in its request to get a second chance to compose a valid request. I have to provide hiim such hints

Whatever , external lib's exceptions (mostly) never specifies which field is the source of aborting the request.

What is the best way, according to you, to handle those exceptions?

My first try at handling those exceptions was to "wrap" the Request class with mine. Each setters are then wrapped in a method which does only one thing : a try/catch block. The catch block then throws a new exceptions of mine : my_out_of_range_volume, or my_out_of_range_price depending on the setter. For instance SetVolume() will be wrapped this way:

My_Request::SetVolume(const int volume) 
{
    try
    {
        m_Request.SetVolume(volume);
    }
    catch(external_lib::out_range_error& e)
    {
        throw my_out_of_range_volume(volume, e);
    }
}

What do you think of it? What do you think about the exception handling overhead it implies? ... :/

Well the question is open, I need new idea to get rid of that lib constraints!

Answer 1

In your first version, you could report the number of operations that succeeded provided the SetXXX functions return some value. You could also keep a counter (which increases after every SetXXX call in that try block) to note what all calls succeeded and based on that counter value, return an appropriate error message.

The major problem with validating each and every step is, in a real-time system -- you are probably introducing too much latency.

Otherwise, your second option looks like the only way. Now, if you have to write a wrapper for every library function and why not add the validation logic, if you can, instead of making the actual call to the said library? This IMO, is more efficient.

Answer 2

If there really are a lot of methods you need to call, you could cut down on the code using a reflection library, by creating just one method to do the calling and exception handling, and passing in the name of the method/property to call/set as an argument. You'd still have the same amount of try/catch calls, but the code would be simpler and you'd already know the name of the method that failed.

Alternatively, depending on the type of exception object that they throw back, it may contain stack information or you could use another library to walk the stack trace to get the name of the last method that it failed on. This depends on the platform you're using.

Answer 3

I think in this case I might dare a macro. Something like (not tested, backslashes omitted):

#define SET( ins, setfun, value, msg )                           
  try {
    ins.setfun( value );
  }
  catch( external::error & )  {
    throw my_explanation( msg, value );
  }

and in use:

Instrument i;
SET( i, SetExpiry, "01-01-2010", "Invalid expiry date" );
SET( i, SetPeriod, 6, "Period out of range" );

You get the idea.

Answer 4

I always prefer a wrapper whenever I'm using third party library. It allows me to define my own exception handling mechanism avoiding users of my class to know about external library. Also, if later the third party changes the exception handling to return codes then my users need not be affected.

But rather than throwing the exception back to my users I would implement the error codes. Something like this:

class MyRequest
{
    enum RequestErrorCode
    {
     PRICE_OUT_OF_LIMIT,
     VOLUME_OUT_OF_LIMIT,
     ...
     ...
     ...
    };

    bool SetPrice(const int price , RequestErrorCode& ErrorCode_out);

    ...

private:

    external_lib::Request mRequest;


};

bool MyRequest::SetPrice(const int price , RequestErrorCode& ErrorCode_out)
{
    bool bReturn = true;
    try
    {

     bReturn = mRequest.SetPrice(price);
    }
    catch(external_lib::out_of_range_error& e)
    {

     ErrorCode_out = PRICE_OUT_OF_LIMIT;
     bReturn = false;
    }
    return bReturn;
}



      bool SendRequest(UserRequest user_request)
{
    MyRequest my_request;
    MyRequest::RequestErrorCode anErrorCode;
    bool bReturn = my_request.SetPrice(user_request.price, anErrorCode);
    if( false == bReturn)
    {
     //Get the error code and process
     //ex:PRICE_OUT_OF_LIMIT
    }
}

Answer 5

Although this is not really the answer you are looking for, but i think that your external lib, or you usage of it, somehow abuses exceptions. An exception should not be used to alter the general process flow. If it is the general case, that the input does not match the specification, than it is up to your app to valid the parameter before passing it to the external lib. Exceptions should only be thrown if an "exceptional" case occurrs, and i think whenever it comes to doing something with user input, you usually have to deal with everything and not rely on 'the user has to provide the correct data, otherwise we handle it with exceptions'.

nevertheless, an alternative to Neil's suggestions could be using boost::lambda, if you want to avoid macros.