Description: ------------ I sent this email to pear-dev@ on Tue, 3 Jun 2003, complete with a patch. I'm submitting this PR per Lukas Smith's advice. I can email the patch again if it gets munged: The fact that the creation process is different across connect() and factory() has always bothered me, plus, there's no need to have the code twice: one incarnation will inevitably suffer, as evidenced by the contents of factory(), with poorer error message etc. Note: passing the whole $options array to factory() is not neccessary but this is the simplest patch that still works. If you want me to change this, let me know. Log message: DB::connect() uses DB::factory() to instantiate the appropriate class. Body of factory() was replaced with instantiation code from connect() (better error message, etc). factory() now takes two arguments, the second being $options (passed through from connect()); it's used just as it used to be in connect(). Reproduce code: --------------- --- DB/DB.php 7 May 2003 16:54:45 -0000 1.20 +++ DB/DB.php 3 Jun 2003 11:32:47 -0000 @@ -216,21 +216,29 @@ * * @param string $type database type, for example "mysql" * + * @param mixed $options see connect() for description + * * @return mixed a newly created DB object, or a DB error code on * error * * access public */ - function &factory($type) + function &factory($type, $options = false) { - @include_once("DB/${type}.php"); + if (is_array($options) && isset($options["debug"]) && + $options["debug"] >= 2) { + // expose php errors with sufficient debug level + include_once "DB/${type}.php"; + } else { + @include_once "DB/${type}.php"; + } $classname = "DB_${type}"; - if (!class_exists($classname)) { - return PEAR::raiseError(null, DB_ERROR_NOT_FOUND, - null, null, null, 'DB_Error', true); + return PEAR::raiseError(null, DB_ERROR_NOT_FOUND, null, null, + "Unable to include the DB/{$type}.php file for `$dsn'", + 'DB_Error', true); } @$obj =& new $classname; @@ -270,22 +278,10 @@ } $type = $dsninfo["phptype"]; - if (is_array($options) && isset($options["debug"]) && - $options["debug"] >= 2) { - // expose php errors with sufficient debug level - include_once "DB/${type}.php"; - } else { - @include_once "DB/${type}.php"; + $obj =& DB::factory($type, $options); + if (DB::isError($obj)) { + return $obj; } - - $classname = "DB_${type}"; - if (!class_exists($classname)) { - return PEAR::raiseError(null, DB_ERROR_NOT_FOUND, null, null, - "Unable to include the DB/{$type}.php file for `$dsn'", - 'DB_Error', true); - } - - @$obj =& new $classname; if (is_array($options)) { foreach ($options as $option => $value) {

that's lame. exactly the same situation we were before. how about putting it this way: "it duplicates code and makes sure factory() keeps bitrotting." how many calls to DB::connect() are typically done that such an optimization is needed? either mark this "won't fix" or leave it "analyzed".

(danielc changed status to "won't fix") thanks for answering my questions, by the way.