Paul M Foster schreef: > On Wed, Jan 21, 2009 at 11:37:07AM -0600, Jay Moore wrote: > >> This is a MySQL class I use and I wanted to get everyone's thoughts on >> how/if I can improve it. This is for MySQL only. I don't need to make >> it compatible with other databases. I'm curious what you all think. >> I'd try to move to using the mysqli functions or class, which you can still wrap in a custom object (I do this because I like to minimize the interface to the bare, bare minimum ... professional laziness you might say). does you class need to be php4 compatible? I'd hope not but you may still have to support php4 ... even then I'd doubt you'd be using php4 for new project so it might be worth making a new php5 only class. ... >> Thanks, >> Jay >> >> Class: >> ------ >> <?php >> >> // Standard MySQL class >> class do_mysql >> { >> // Constructor >> function __construct() >> { >> $this->do_mysql(); >> } >> >> // Destructor >> function __destruct() >> { >> //$this->close(); >> } >> >> function do_mysql() >> { >> $this->login = ''; >> $this->pass = ''; >> >> $this->link = @mysql_connect('localhost', $this->login, >> $this->pass) >> or die('Could not connect to the database.'); >> } // End do_mysql >> >> // Functions >> function close() >> { >> if ($this->link) >> { >> mysql_close($this->link); >> unset($this->link); >> } >> } // End close >> >> function fetch_array() >> { >> return mysql_fetch_array($this->result); >> } // End fetch_array >> >> function last_id() >> { >> return mysql_insert_id($this->link); >> } // End last_id >> >> function num_rows() >> { >> return mysql_num_rows($this->result); >> } // End num_rows >> >> function process($database = '') >> { >> if (is_null($this->query)) >> { >> die('Error: Query string empty. Cannot proceed.'); >> } >> >> $this->db = @mysql_select_db($database, $this->link) >> or die("Database >> Error: Couldn't select $database <br />" . mysql_error()); >> $this->result = @mysql_query($this->query, $this->link) or >> die('Database Error: Couldn\'t query. <br />' . mysql_error() . "<br >> /><br /> $this->query"); >> } // End process >> >> function sanitize(&$ref) >> { >> $ref = mysql_real_escape_string($ref); >> } // End sanitize >> >> } // End do_mysql >> >> ?> >> >> >> Sample usage: >> $value = 'value'; >> $sql = new do_mysql(); >> $sql->sanitize($value); >> $sql->query = "SELECT * FROM `wherever` WHERE `field` = '$value'"; >> $sql->process('dbname'); >> $sql->close(); >> >> if ($sql->num_rows()) >> { >> while ($row = $sql->fetch_array()) >> { >> do stuff; >> } >> } >> > here's another (php5) version of your class, see what you think: <?php class do_mysql { private $link; private $result; private $db; // Constructor function __construct($login, $pass, $db, $server = 'localhost') { $this->link = mysql_connect($server, $login, $pass); if (!$this->link) throw new Exception('Could not connect to the database.'); $this->setDB($db); } // Destructor function __destruct() { $this->close(); } // Functions function close() { if (!$this->link) return; mysql_close($this->link); unset($this->link); } function fetch_array() { return mysql_fetch_array($this->result); } function last_id() { return mysql_insert_id($this->link); } function num_rows() { return mysql_num_rows($this->result); } function setDB($database) { if (!$database || !mysql_select_db($database, $this->link)) throw new Exception("Database Error: Couldn't select $database - " . mysql_error()); } function query($query) { if (empty($query)) throw new Exception('Error: Query string empty. Cannot proceed.'); $this->result = mysql_query($this->query, $this->link); if (!$this->result) // beware that putting this msg on screen is a security hazard throw new Exception('Database Error: Couldn\'t query - ' . mysql_error() . " - $query"); } function escape($ref) { return mysql_real_escape_string($ref, $this->link); } } > A couple of thoughts. First precede all your mysql_* calls with the at > sign (@) to shut up the routines if they generate text. I had this > problem, and that was the answer. yes that's very bad advice. using error suppression is bad for performance and debugging, don't do it unless you really really have to (e.g. you have some function that spits warnings even with display_errors set to Off) display_errors should be set to Off in production, and errors/warnings shouldn't be suppressed, they should be logged. handle errors gracefully ('or die()' is not graceful) > Second, store your connection resource as a class variable, so you can > pass it around to the various routines. Actually, you're already doing > this, but I prefer to do so explicitly, as: > > var $link; that's very php4. > at the top of the class. > > I have a similar class for PostgreSQL. I also have routines like > "update", which allow you to pass a table name and an associative array > of field values. Same thing for an "insert" routine. if the postgres extension is anything like the firebird extension then there may actually be a few calls which do require error suppression :-) > Paul > -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php