MySQL using OO PHP

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

MichaelR
Forum Contributor
Posts: 148
Joined: Sat Jan 03, 2009 3:27 pm

MySQL using OO PHP

Post by MichaelR »

This is my first attempt at object-oriented PHP coding, so any critique would be appreciated. I've tested the code and know that it works, but I'm unsure about whether or not the style is good or bad (am I going against best practices?).

Note: After reading the Coding Critique post, I've followed the advice and updated this first post with the newest copy of the code.

Code: Select all

<?php
 
  class MySQL {
 
    private $connection;
 
    public function __construct($array = false) {
 
      if (!is_array($array)) {
 
        $array   = array(
 
          0 => 'hostname',
          1 => 'username',
          2 => 'password',
          3 => 'database',
 
        );
 
      }
 
      $this->connect($array);
 
    }
 
    private function connect($array) {
 
      $this->connection = @mysql_connect($array[0], $array[1], $array[2]);
 
      if (!empty($array[3])) {
        $this->database($array[3]);
      }
 
    }
 
    public function connected() {
      return is_resource($this->connection);
    }
 
    public function database($database) {
 
      if (!$this->connected()) {
        return false;
      }
 
      return @mysql_select_db($database, $this->connection);
 
    }
 
    public function query($query) {
 
      if (!$this->connected()) {
        return false;
      }
 
      return new MySQLResult(@mysql_query($query, $this->connection));
 
    }
 
    public function disconnect() {
 
      if (!$this->connected()) {
        return false;
      }
 
      return @mysql_close($this->connection);
 
    }
 
  }
 
  class MySQLResult {
 
    private $result;
 
    public function __construct($result) {
      $this->result = $result;
    }
 
    public function fetch() {
 
      if (!is_resource($this->result)) {
        return false;
      }
 
      return @mysql_fetch_assoc($this->result);
 
    }
 
  }
 
?>
Last edited by MichaelR on Thu Sep 10, 2009 6:35 am, edited 31 times in total.
User avatar
Darhazer
DevNet Resident
Posts: 1011
Joined: Thu May 14, 2009 3:00 pm
Location: HellCity, Bulgaria

Re: MySQL using OO PHP

Post by Darhazer »

Your error handling is not OO... Make $connection protected member, and throw exceptions when you cannot connect or execute query.
Pulni4kiya
Forum Commoner
Posts: 35
Joined: Tue Apr 14, 2009 6:20 am

Re: MySQL using OO PHP

Post by Pulni4kiya »

The normal thing is the MySQL class to extend the Database class or even better - implement some DB interface.
Then in your project you use variables of that interface and can always add other implementations of the interface and change which implementation your project uses.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Re: MySQL using OO PHP

Post by Jenk »

Darhazer wrote:Make $connection protected member
Why?
User avatar
m4rw3r
Forum Commoner
Posts: 33
Joined: Mon Aug 03, 2009 4:19 pm
Location: Sweden

Re: MySQL using OO PHP

Post by m4rw3r »

Jenk wrote:
Darhazer wrote:Make $connection protected member
Why?
Because it isn't good if a user can access it directly.
If the user of the class accesses it directly, it means that the user must use database specific functions on that property (eg. mysql_query('SELECT * ...', $db->connection), simple example).

That means that the user must know what that property is, and how it should be treated.
And if you eg. change the db functions in the class, you would end up with a resource which is incompatible with the functions/methods the user of the class applies on that property.

(And if the user replaces that property... *shudder*)

The benefits of a public property is that it can be modified, but if the class is coded properly the user won't need to modify it or access it.
towerspl
Forum Newbie
Posts: 20
Joined: Mon Aug 03, 2009 7:37 am

Re: MySQL using OO PHP

Post by towerspl »

yep you should for best practice have all variables that you would not like to be directly modified set to private or public
User avatar
jayshields
DevNet Resident
Posts: 1912
Joined: Mon Aug 22, 2005 12:11 pm
Location: Leeds/Manchester, England

Re: MySQL using OO PHP

Post by jayshields »

Don't use the error surpressor operator (@).
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: MySQL using OO PHP

Post by Christopher »

I think I would move query() into the "MySQL" class and have it return a "database" class object as the result. The add Iterator support to that result class.
(#10850)
MichaelR
Forum Contributor
Posts: 148
Joined: Sat Jan 03, 2009 3:27 pm

Re: MySQL using OO PHP

Post by MichaelR »

jayshields wrote:Don't use the error surpressor operator (@).
Why not? I've accounted for a failed connection on line 41. I'd prefer to output my own error reports than use those which are in-built.
Darhazer wrote:Your error handling is not OO... Make $connection protected member, and throw exceptions when you cannot connect or execute query.
The problem I have with this is that I can't see how to make it work for my intended method. I'm going for a completely modular site, where all the content is added through includes. For example:

Code: Select all

 
 
 if (!$database->connection) {
  include('error.php');
 }
 
 else {
  include('home.php');
 }
 
 
Can this be done using thrown exceptions?
Last edited by MichaelR on Wed Aug 19, 2009 7:50 am, edited 1 time in total.
User avatar
Eran
DevNet Master
Posts: 3549
Joined: Fri Jan 18, 2008 12:36 am
Location: Israel, ME

Re: MySQL using OO PHP

Post by Eran »

I'd suggest moving to the MySQLi extension
MichaelR
Forum Contributor
Posts: 148
Joined: Sat Jan 03, 2009 3:27 pm

Re: MySQL using OO PHP

Post by MichaelR »

Note: Moved to the original post.
Last edited by MichaelR on Fri Aug 21, 2009 5:29 pm, edited 39 times in total.
User avatar
Eran
DevNet Master
Posts: 3549
Joined: Fri Jan 18, 2008 12:36 am
Location: Israel, ME

Re: MySQL using OO PHP

Post by Eran »

It's somewhat pointless to declare static private properties that have no accessors. Define them as class constants instead (and so they will be available to extending classes as well).
MichaelR
Forum Contributor
Posts: 148
Joined: Sat Jan 03, 2009 3:27 pm

Re: MySQL using OO PHP

Post by MichaelR »

pytrin wrote:It's somewhat pointless to declare static private properties that have no accessors. Define them as class constants instead (and so they will be available to extending classes as well).
I've changed it slightly since you posted this, but there was a reason for using them as I had. It was only to be used within that class (hence private), but was to be the same throughout every instantiation of it (hence static).
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Re: MySQL using OO PHP

Post by Jenk »

m4rw3r wrote:
Jenk wrote:
Darhazer wrote:Make $connection protected member
Why?
Because it isn't good if a user can access it directly.
If the user of the class accesses it directly, it means that the user must use database specific functions on that property (eg. mysql_query('SELECT * ...', $db->connection), simple example).

That means that the user must know what that property is, and how it should be treated.
And if you eg. change the db functions in the class, you would end up with a resource which is incompatible with the functions/methods the user of the class applies on that property.

(And if the user replaces that property... *shudder*)

The benefits of a public property is that it can be modified, but if the class is coded properly the user won't need to modify it or access it.
and what if I wanted to share this objects connection with another class/object, but not share the behaviour of this object?
User avatar
Darhazer
DevNet Resident
Posts: 1011
Joined: Thu May 14, 2009 3:00 pm
Location: HellCity, Bulgaria

Re: MySQL using OO PHP

Post by Darhazer »

Actually my post was:
Change private $connection to protected $connection
And it have nothing to do with public properties
The difference between private and protected is that if you extends the class, the subclass (the extended class) will still have the property $connection (if the property is protected).
So if you have no good reason to declare something as private, do not do that
Otherwise you or someone else will extend the class, and will have hard time catching why the inherited methods do not work

Edit: since the original code is modified, it's possible that my post was about public => protected, but now I would tell about private => protected
Last edited by Darhazer on Sat Aug 22, 2009 5:55 pm, edited 1 time in total.
Post Reply