Practical PHP Refactoring: Replace Error Code with Exception
Join the DZone community and get the full member experience.
Join For FreeError codes are mostly a relic of procedural programming: the object-oriented paradigm has introduced exceptions as a first-class mechanism to separate error handling from. This refactoring targets special error values returned by methods and transform them into exceptions.
Fowler talks about a fundamental distinction of responsibility from finding out an error is occured and doing something about it, like displaying an error message. As such, these two responsibilities are placed in wildly different locations in your code; for example, when the database server is unavailable for some reason, a very lower layer like the PDO object detects the error, while you can handle it by serving an error page in the component of your application that products the presentation (HTML code).
Exceptional advantages
Exceptions avoid continuous if() statements inserted to check the return value of a method: in case of an error, the exception bubbles up to the nearest catch clause without the intermediate code knowing about its existence:
try { $object->method(); } catch (SomeException $e) { /* ... */ }
The stack trace in this case contains many methods called by $object internally. Only the ones throwing the exception know that the SomeException class exists: methods that do not throw it are oblivious to this fact. Also methods that call other methods that in turn throw exceptions are not required to know (all PHP exceptions are unchecked and as such PHP's RuntimeException is a useless class):
public function method() { $this->collaborator->otherMethod(); // this call may throw an exception, but this fact can be decoupled from this method }
Thus a lot of code is less coupled to the exception, and changing or expanding it will be easier.
Compare this freedom with the error code approach; you would have two choices.
- Either every method checks the return value of the called method with an if() (for every call where an error is possible).
- Or you will be forced to deal with any error immediately:
mysql_connect($host, $user, $password) or die('...');
There are other few advantages of exceptions over error codes returned by methods:
- exception objects can implement methods and so are a way to avoid Primitive Obsession. For example, their can implement methods that return a translated version of the error message.
- Exceptions can be used in constructors to signal an error, while a new() call cannot return a value different from the instance.
Exceptions can easily be abused
Exceptions are reserved for error handling: raising and catching them for other purposes is a goto where the label is dynamic. The exception will bubble up until it finds a catch, and resume execution from there. Throw exceptions only for real error conditions, not for things that happens very often.
In my opinion validation is the limit for exceptions usage: any more ordinary use case and exceptions must not be used.
Steps
- Find all calls and adjust them to use a try/catch block instead of checking the return value. Later you will be able to remove most of these constructs if there is an upper layer managing errors. You will be able to delete much code if there is a long trace of methods between the error generation and its handling.
- To make the tests pass again, throw the exception instead of returning the special error value.
- Change the signature of the method: add @throws clauses for documentation.
if it is too much to change (too many calls), Fowler suggests to tackle one call at the time by duplicating the original method and maintaining two versions for a bit of time (some commits).
I add that you should also target one error code at the time: multiple error code values call for different checking routines and usually for multiple exception classes. You can always extract an interface or a superclass between different exception classes to be able to catch all of them with a single block.
Example
In the initial state, a Bag object has to be filled with items. In this example I'm modelling just the aspect making sure it is not too full, and not maintaining an internal collection of items.
The tests check the happy path (no problem with the weight) and a particular failure path (too much weight is put in our Bag, which will break).
<?php class ReplaceErrorCodeWithException extends PHPUnit_Framework_TestCase { public function testItemsCanBeAddedAtWill() { $bag = new Bag(10); $result = $bag->addItem('Domain-Driven Design', 10); $this->assertEquals(0, $result); $this->assertEquals(10, $bag->getWeight()); } public function testTheWeightLimitCannotBeInfringed() { $bag = new Bag(10); $result = $bag->addItem('Land of Lisp', 11); $this->assertEquals(Bag::TOO_MUCH_WEIGHT, $result); $this->assertEquals(0, $bag->getWeight()); } } class Bag { private $weightLimit; private $weight; const TOO_MUCH_WEIGHT = 1; public function __construct($weightLimit) { $this->weightLimit = $weightLimit; $this->weight = 0; } /** * @return int 0 if no errors */ public function addItem($name, $itemWeight) { if ($this->weightLimit < $this->weight + $itemWeight) { return self::TOO_MUCH_WEIGHT; } $this->weight += $itemWeight; return 0; } public function getWeight() { return $this->weight; } }
We now want to refactor to an exception, so we change the client code calling the method under refactoring. The two tests represent this code in the example.
<?php class ReplaceErrorCodeWithException extends PHPUnit_Framework_TestCase { public function testItemsCanBeAddedAtWill() { $bag = new Bag(10); $result = $bag->addItem('Domain-Driven Design', 10); $this->assertEquals(10, $bag->getWeight()); } public function testTheWeightLimitCannotBeInfringed() { $bag = new Bag(10); try { $result = $bag->addItem('Land of Lisp', 11); $this->fail('Adding the item should not be allowed.'); } catch (TooMuchWeightException $e) { $this->assertEquals(0, $bag->getWeight()); } } }
The first test still passes, but does not check the return value anymore as the default case is the good one. The second test uses try/catch and a $this->fail() call to check the exception presence; we could have used a @expectedException annotation for PHPUnit, but not in this case where we need to make an assertion on the state of the Bag instance after the error.
Now the contract between client code and this class is broken. I don't advise to commit here; first, let's fix the error raising by adding the required class and the throwing code.
class Bag { private $weightLimit; private $weight; const TOO_MUCH_WEIGHT = 1; public function __construct($weightLimit) { $this->weightLimit = $weightLimit; $this->weight = 0; } /** * @return int 0 if no errors */ public function addItem($name, $itemWeight) { if ($this->weightLimit < $this->weight + $itemWeight) { throw new TooMuchWeightException("Weight has exceeded the limit for this Bag: $this->weightLimit"); } $this->weight += $itemWeight; return 0; } public function getWeight() { return $this->weight; } } class TooMuchWeightException extends Exception {}
Now the tests pass again. We can update the docblock of the class and delete unnecessary code (such as the constant).
class Bag { private $weightLimit; private $weight; public function __construct($weightLimit) { $this->weightLimit = $weightLimit; $this->weight = 0; } /** * @throws TooMuchWeightException */ public function addItem($name, $itemWeight) { if ($this->weightLimit < $this->weight + $itemWeight) { throw new TooMuchWeightException("Weight has exceeded the limit for this Bag: $this->weightLimit"); } $this->weight += $itemWeight; return 0; } public function getWeight() { return $this->weight; } }
Opinions expressed by DZone contributors are their own.
Comments