refactor 'execute and log' pattern

Question

I found myself keep following the same pattern again and again:

if((return_code = doFoo1(...)) != CODE_OK) {
    log("useful log message based on return code");
    // very likely to return an error code. 
}

// continue

if((return_code = doFoo2(...)) != CODE_OK) {
    log("useful log message based on return code");
    // very likely to return an error code. 
}

Any insight how to avoid this annoying pattern and keep the logic clean?

Answer 1

There are a number of ways to tackle this, here's a basic one:

bool CheckAndLog( int code )
{
  if( code == CODE_OK )
    return true;
  log( "<some error based on code>" );
  return false;
}

if( !CheckAndLog( doFoo1(...) ) )
  return;

If you also need the return code for later use you could return it from CheckAndLog, or have it as a pass-by-reference argument, or even make CheckAndLog into a class which stores the latest error code and which you instantiate with the (possibly scoped) logger instance to use.

update if you want file and line info you need the __FILE__ and __LINE__ macros and to save yourself from typing them all over the place a simple macro is sufficient:

bool CheckAndLog( int code, const char* file, unsigned line )
{
  if( code == CODE_OK )
    return true;
  std::cerr <<  "file " << file << " line " << line
            << "<errormessage>" << std::endl;
  return false;
}

#define CHECK( what ) (CheckAndLog( what, __FILE__, __LINE__ ))

Take it one step further to get the called function into that: since it is available as the macro argument it is expanded as such by the preprocessor. Meaning if you type CHECK( foobar( 65 ) ) the what argument is seen literally as foobar( 65 ) and not as it's return value. Which is ideal since the preprocessor can also turn that into a string:

bool CheckAndLog( int code, const char* desc, const char* file, unsigned line )
{
  if( code == CODE_OK )
    return true;
  std::cerr <<  desc << " from file " << file << " line " << line
            << "<errormessage>" << std::endl;
  return false;
}

#define STRINGIZE1( x ) #x
#define STRINGIZE( x ) STRINGIZE1( x )
#define CHECK( what ) (CheckAndLog( what, STRINGIZE( what ), __FILE__, __LINE__ ))

I'd advise against using CHECK as the name for your macro though since it is more often than not used by unit test libraries as well. Also take great care never to redefine your macro (by accident or on purpose) to something like #define CHECK( what ) (true) because that would mean if you write CHECK( foobar( 65 ) ), the function will simply never be called.