Friday, 8 April 2011

Conceptual Issues on Error Handling and Defensive Coding


1. Try/Catch

Try/Catch should be used for instructions that access hardware, a instruction that does use the Hard Drive or heavy CPU usage, should use try catch. All the rest should be treated as normal because most of the functions that deals with hardware already have a built-in try/catch.

Exceptions are thrown - they are intended to be caught. Errors are generally unrecoverable. Let’s say you have a block of code that will insert a row into a database. It is possible that this call fails (duplicate ID for example) - you will want to have a "Error" which in this case is an "Exception". When you are inserting these rows, you can do something like this

try {
  $row->insert();
  $inserted = true;
} catch (Exception $e) {
  echo "There was an error inserting the row - ".$e->getMessage();
  $inserted = false;
}

echo "Some more stuff";

Program execution will continue - because you 'caught' the exception. An exception will be treated as an error unless it is caught. It will allow you to continue program execution after it fails as well. Putting in another way, exceptions are meant to handle errors (at least in PHP). Suppose you are in a routine, and an error is occurred that you can't handle in the current context.


/**
* @throws Exception_NoFile
*/
function read_file($file) {
  if(!file_exists($file)) {
    throw new Exception_NoFile($file);
  }
  /* ... nominal case */
}

In this situation you can't continue with the nominal case, because there is no file to process. You have to choose:

• return with an invalid return value (this is the C practice, e.g: return -1 or using status flags)

• throw an exception, and hope, someone will catch it above. If your client code excepts it, no problem, it may try another path or re-throw an exception. If your client isn't ready to handle those situations where the requested file doesn't exist... your code will fail with an un-cached exception, as it would do with a read of a non-existing file in the other approach.

2. Exception Propagation

What guidelines to apply when deciding whether to let a method exception propagate up or handle it once the exception is received? Example: Suppose three methods method1, 2, and 3. Method1 calls Method2 which Calls Method3 and the exception is only thrown in method 3. When should you let the exception propagate upward as follows:

method1 {
  try {
    call method2;
  } catch (exception e) {
    doErrorProcessing;
  }
}

method2 throws exception {
  call method3;
}

method3 throws exception {
  call readFile;
}

And when should I handle the exception once it is raised as follows

method1 {
  call method2;
}
method2 {
  call method3;
}

method3 {
  try {
    call readFille
  } catch (exception e) {
    doErrorProcessing;
  }
}

Like all answers in software, this one for me is "it depends". If I can fix an exception, or nullify the problem that caused it (sometimes this means just ignoring it altogether), I'll handle it. Otherwise it gets passed up to the next level. And I always try to fix exceptions as low in the tree as possible (i.e. as soon as possible after they occur) - this localizes exception handling and avoids big honkin' exception handlers in your upper levels.

Most exceptions should be, in fact, exceptional, so generally I tend to like them to break the app when they occur. If the exception represents some recoverable error condition, then it should be handled at the point in the call stack when you have the information you need to do so safely. This would include situations where the error can be corrected by the user, i.e. catching the exception to report it back to the user so that they can take appropriate actions.

There's nothing wrong with a function throwing 10 different kind of exception, as long as each exception makes sense in the context. If you just want to detect that something went wrong but you don't need to know exactly what, you can stick with your approach (ie throwing only one exception). On the other hand, if you need to react to the different problems, you should keep the original exception.

There's no "better" approach, this depends on the context and the information you need to have on the upper level. For instance, if you have a getAThingy function and your application can be configured such that the thingy in question can come from a database or from a file, it's probably not appropriate for getAThingy to raise SqlException in the database case, because then code calling it has to handle that whether configured for files or database. (It would become a leaky abstraction.) So that would be an example of when you should probably catch and throw something else more appropriate.

In contrast, if you accept an identifier of some kind that shouldn't be null and someone passes in null, it seems perfectly appropriate to happily pass on the NullPointerException without wrapping it.

3. Exceptions X Errors

Use exceptions for things that are truly beyond your control. A good example would be:

try {
  if (fopen('file.txt', 'w') === false) {
    throw new Exception('File could not be opened for write access.');
  }
  } catch (Exception $e) {
  echo $e->getMessage();
}

Bad:

try {
  if (strlen($_POST['username']) < 5) {
    throw new Exception('Username too short');
  }
  } catch (Exception $e) {
  echo $e->getMessage();
}

The first way is good because it occurs when its something that the user or the application cant control. It cant open the file because? Could be many reasons. The second way is an overkill use of try /catch when you should use trigger_error. The second way is down to the user not knowing the rules of the username validation.

In short use exceptions when you cant control what your testing. Remember exceptions have more overhead then trigger_error.

4. Defensive Programming

How much defensive checking should you have on your functions? How do you handle method parameters and call orders? Suppose methods a, b, and c. You can call a>b, a>c, and a>b>c, but anything else, such as, b>a, c>a, c>a>b, etc. If this will break the code, how do you go out of your way to catch these mistakes and let the user know? Should you let it break naturally and just leave them to the documentation and say, if they're not using it as documented not much I can do?

If methods must be called in a specific order, calling them in another order should cause an error. The data will not be correct; in this case, well, it'll fail and the developer will see that as soon as he tests his code for the first time; so, I'd say it's something that's not likely to happen on the production server, and wouldn't put too much code to test for it. More code means more risk of bugs.

5. Errors on the Constructor

A constructor in PHP will always return void. This will not work:

public function __construct()
{
  return FALSE;
}

Throwing an Exception in the constructor

public function __construct($camperId)
{
  if($camperId === 1) {
    throw new Exception('ID 1 is not in database');
  }
}

Would terminate script execution unless you catch it somewhere...

try {
  $camper = new SummerCamper(1);
  } catch(Exception $e) {
  $camper = FALSE;
}

You could move the above code into a static method of SummerCamper to create instances of it instead of using the new keyword:

class SummerCamper
{
  protected function __construct($camperId)
  {
    if($camperId === 1) {
    throw new Exception('ID 1 is not in database');
  }
}

public static function create($camperId)
{
  $camper = FALSE;
  try {
    $camper = new self($camperId);
  } catch(Exception $e) {
  // uncomment if you want PHP to raise a Notice about it
  // trigger_error($e->getMessage(), E_USER_NOTICE);
  }
  return $camper;
  }
}

This way you could do

$camper = SummerCamper::create(1);

and get FALSE in $camper when the $camper_id does not exist. It would be good for this situations to use a Factory instead.

No comments:

Post a Comment