| Package home | Report new bug | New search | Development Roadmap | Status: Open | Feedback | All | Closed Since Version 1.12.3 |
| Bug #25 | DB::factory() left to bitrot, DB::connect() duplicates code | ||
|---|---|---|---|
| Submitted: | 2003-09-21 20:48 UTC | ||
| From: | neuhauser at bellavista dot cz | Assigned: | danielc |
| Status: | Wont fix | Package: | DB |
| PHP Version: | 4.3.3 | OS: | FreeBSD 4.8-STABLE |
| Roadmaps: | (Not assigned) | ||
[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) {
Comments
[2003-12-29 13:10 UTC] User who submitted this comment has not confirmed identity
If you submitted this note, check your email.If you do not have a message, click here to re-send MANUAL CONFIRMATION IS NOT POSSIBLE. Write a message to pear-dev@lists.php.net to request the confirmation link. All bugs/comments/patches associated with this email address will be deleted within 48 hours if the account request is not confirmed!
[2004-01-14 22:52 UTC] User who submitted this comment has not confirmed identity
If you submitted this note, check your email.If you do not have a message, click here to re-send MANUAL CONFIRMATION IS NOT POSSIBLE. Write a message to pear-dev@lists.php.net to request the confirmation link. All bugs/comments/patches associated with this email address will be deleted within 48 hours if the account request is not confirmed!
[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".