c2oo / xtrabackup-manager

Automatically exported from code.google.com/p/xtrabackup-manager
0 stars 0 forks source link

TODO: Use Exception paradigm instead of signaling error in return value #27

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
XBM code is written in the classic C/PHP style where errors are signaled by 
returning "false" or other similar value. The caller must check the return 
value from each function call and if necessary handle the error. Example:

scheduledBackup.class.php:

                       if( ! ($res = $conn->query($sql) ) ) {
                                $this->error = 'scheduledBackup->pollHasValidSeed: '."Error: Query: $sql \nFailed with MySQL Error: $conn->error";
                                return false;
                        }

                        if( $res->num_rows > 1 ) {
                                $this->error = 'scheduledBackup->pollHasValidSeed: '."Error: Found more than one valid seed for this backup. This should not happen.";
                                return false;
                        } elseif( $res->num_rows == 1 ) {

Unfortunately, this style makes the code hard to read.

An added complication is inside constructor methods of many classes, since it 
is not possible to return a value from the constructor. In the following 
example the constructor accepts any value, and errors are only checked and 
acted on later. In the worst case the same check needs to be done in many 
different places. All of this is bad design:

mysqlType.class.php

                function __construct($id) {
                        $this->id = $id;
                        $this->log = false;
                }

                function getInfo() {

                        global $config;

                        if(!is_numeric($this->id)) {
                                $this->error = 'mysqlType->getInfo: '."Error: The ID for this object is not an integer.";
                                return false;
                        }
                        ...

The preferable method is to instead use the exception paradigm, supported in 
PHP 5. For instance, the previous code would then be better written like this:

                function __construct($id) {
                        if(!is_numeric($id)) {
                                throw new Exception('mysqlType->getInfo: '."Error: The ID given is not an integer.");
                        }
                        $this->id = $id;
                        $this->log = false;
                }

                function getInfo() {

                        global $config;
                        ...

Similarly, from the calling function, it is possible to run through several 
statements at once, and catch any potential errors at the end of the code 
block, not between each statement:

try {
 function1();
 function2();
 function3();
catch(Exception $e) {
  print basename(__FILE__) . ': ' . $e->getMessage() . "\n";
  die();
}

Unfortunately, core PHP functions do not throw exceptions, so errors need to be 
checked the traditional way still.

Original issue reported on code.google.com by henrik.i...@gmail.com on 1 Apr 2011 at 2:07

GoogleCodeExporter commented 8 years ago
Refactored the code today to use exception paradigm.. I love the improved ease 
of core writing!

Committed in revision 26.

Feel free to review?

Original comment by lachlan....@gmail.com on 1 Apr 2011 at 9:35