Home » Database » DB » Bug #25

[2003-09-21 20:48 UTC] neuhauser at bellavista dot cz

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) {

[2004-01-15 10:32 UTC] cynic at php dot net

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".

[2004-01-15 13:53 UTC] cynic at php dot net

(danielc changed status to "won't fix")

thanks for answering my questions, by the way.