Rik's Ramblings

Monday, December 17, 2012

Exception or Return?


When I first read about programming, I learned that "multiple returns from a function" were a bad idea.

I definitely agree with the sentiment. Most of the memory leaks I see in code are due to functions bailing, such as the example below:

int doWork(int c)
{
  char *memory = (char *)malloc(BIGBLOB);

  if (!do_thing_1(memory))
  {
    return -1;
  }

  int m = do_thing_2(memory);

  int e = m*c*c;

  free(memory);
  
  return e;
}

In the example above (incase you didn't catch it) the memory malloc'ed on the first line of the function ewould be leaked if the call to do_thing_1() returned false.

The pattern I've adopted to avoid these kinds of memory leaks, and properly handle clean-up, is to use exceptions.

I define an error class that can be thrown at the various places within the code, then a single exception handler can interpret the value thrown and print an error message describing what went wrong. The code looks something like this:

bool doBigJob()
{
  bool retval = false; 

  char *memBig = NULL;
  char *b = NULL;
  try
  {

    int a = doThingOne();
    if (-1 == a) throw (Problems::FIRST_PROBLEM);

    int s = getBigNumber();
    memBig = new char[s];

    b = (char *)doThingTwo();
    if (NULL == b) throw (Problems::SECOND_PROBLEM);

    char c = b[a];
    if (c == 'E')
    {
      // Arbitrary reason to abort!
      throw (Problems::THIRD_PROBLEM);
    }

    // Do meaty stuff...
    while (0 != s)
    {
      --s;
      memBig[s] = c;
    }

    retval = true;
  }
  catch(Problems::_problem p)
  {
    std::cout
      << "Failed for reason " 
      << (int)p 
      << std::endl;
  }
  catch(std::bad_alloc)
  {
    // This exception handler is required for new()
    std::cout 
      << "Failed for bad alloc\n";
  }

  // Clean-up, with null checks, 
  // This is safe in the exception cases
  if (NULL != memBig) delete[]memBig;
  if (NULL != b) undoThingTwo();

  return retval;
}

In the example above Problem conveys what went wrong. Actually, Problem is simply an enum, as shown below:

namespace Problems {
  enum _problem
  {
    NO_PROBLEM = 0,
    FIRST_PROBLEM = 1,
    SECOND_PROBLEM = 2,
    THIRD_PROBLEM = 3
  };
}

This pattern is based on a technique I encountered in an old job, where they used GOTO to achieve a similar thing. Of course, people tell you you must never use GOTO, but I actually think the way it is used here is perfectly valid:

bool doBigJob()
{
  bool retval = false; 

  char *memBig = NULL;
  char *b = NULL;

  int a = doThingOne();
  if (-1 == a) goto done;

  int s = getBigNumber();
  memBig = (char *)malloc(s*sizeof(char));

  b = (char *)doThingTwo();
  if (NULL == b) goto done;

  char c = b[a];
  if (c == 'E')
  {
    // Arbitrary reason to abort!
    goto done;
  }

  while (0 != s)
  {
    --s;
    memBig[s] = c;
  }

  retval = true;

:done
  if (NULL != memBig) free(memBig);
  if (NULL != b) undoThingTwo();

  return retval;
}

The thing is, I don't encounter my approach being used anywhere in code I read. People seem quite happy to use the dangerous multiple returns from a function approach.

I know in Java, at one point, it was felt that using exceptions was too expensive and they should be used sparingly. But when I look at the code backing exceptions in C++, I don't think there's any real downside to doing it. I definitely think it is a cleaner way to handle these (exceptional) error cases in the flow of execution of complex functions.

As my code sample suggests, you most likely need to deal with platform exceptions in many places, for example when using the new() operator.

I suppose another approach would be to simply propagate Problems exceptions up to a higher layer. But another problem with C++ is that it's not clear what exceptions are going to get generated by a function, so it's not clear to me that this would make life easier for a caller of a function.

What I need is a good book on the subject. I wonder if this one of Herb's is any good?


0 Comments:

Post a Comment



<< Home