My First Class

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

My First Class

Post by psurrena »

Below is my first real attempt at a functional class. My first question is, am I approaching this correctly? Second, loop() only returns one result when I know there are many more.

This class is for querying the db and returning result:

Code: Select all

<?php
	//QUERY CLASS TEST
	
	mysql_connect('localhost','','');
	mysql_select_db(database);

	class TESTQUERY{
		public $query;
		public $row;
		public $loopdata;
		private $result;

		function query(){	
			$this->result=mysql_query($this->query) or die (mysql_error());
			if(!mysql_numrows($this->result)){
				echo "There is no information available.";
			} else {
				$this->row=mysql_fetch_assoc($this->result);
			}
		}
		
		function loop(){
			$this->result=mysql_query($this->query) or die (mysql_error());
			if(!mysql_numrows($this->result)){
				echo "There is no information available.";
			} else {
				while($this->row=mysql_fetch_assoc($this->result)){
					return $this->loopdata;
				}
			}
		}
		
	}
	
	//Single Row
	$people=new TESTQUERY;
	$people->query="SELECT people_fname FROM people WHERE people_id='$id'";
	$people->query();
	echo $people->row['people_fname']."<br />";
	
	//Loop
	$thing=new TESTQUERY;
	$thing->query="SELECT people_fname FROM people";
	$thing->loop();
	echo $thing->loopdata=$thing->row['people_fname'];
?>
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Post by alex.barylski »

Depends on what your trying to achieve. :)

Firstly, OOP enthusiasts/zealots would never allow public member variables. Use accessor/mutator (get/set) methods instead.

Secondly, your class is bound to mysql as a RDBMS, so I am not sure I see the "purpose" behind your class. Are you trying for a DB abstraction layer like AdoDB, etc?

Thirdly, a quick glance at your code loop() is telling me your returning the minute tested condition is TRUE, so only a single record is returned.

What you want is something like this:

Code: Select all

function loop()
{ 
  $this->result=mysql_query($this->query) or die (mysql_error()); 
  
  if(!mysql_numrows($this->result)){ 
    echo "There is no information available."; 
  }
  else { 
    // Loop over each row and add to array
    while($arr[]=mysql_fetch_assoc($this->result));
    
    return $arr; // Return array
  } 
}
Last but not least. If you want classes to be reusable, you can remove hardcoded error messages and use error codes instead so the client developer can choose/manage the error messages themselves.

HTH

Cheers :)
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

Post by psurrena »

My purpose for this class is not so much portability but to clean up site I'm working on. The procedural code I'm working with has so many repeats that I thought this would be the proper step in simplifying.
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

Post by psurrena »

Oh! And thanks for fixing the loop. Why does $arr not require $this->?
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

I think current practice is to have database connection/query classes to manage the data layer. Then on top of that build some sort of gateway. You might want to think about this interface:

http://www.martinfowler.com/eaaCatalog/ ... teway.html

There have been recent discussions here about this subject that might be of interest to you:

viewtopic.php?t=68300&highlight=dbconnect
Hockey wrote:Firstly, OOP enthusiasts/zealots would never allow public member variables. Use accessor/mutator (get/set) methods instead.
I consider myself quite zealatinous and have no problem with accessing public properties as appropriate. You cannot implement numerous things (ValueObject, ActiveRecord, etc.) without them. However, I am not sure that in this case the query string is appropriate.
(#10850)
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Post by alex.barylski »

arborint wrote:I think current practice is to have database connection/query classes to manage the data layer. Then on top of that build some sort of gateway. You might want to think about this interface:

http://www.martinfowler.com/eaaCatalog/ ... teway.html

There have been recent discussions here about this subject that might be of interest to you:

viewtopic.php?t=68300&highlight=dbconnect
Hockey wrote:Firstly, OOP enthusiasts/zealots would never allow public member variables. Use accessor/mutator (get/set) methods instead.
I consider myself quite zealatinous and have no problem with accessing public properties as appropriate. You cannot implement numerous things (ValueObject, ActiveRecord, etc.) without them. However, I am not sure that in this case the query string is appropriate.
I personally have never found it nessecary to expose properties directly, but I have never implemented a ValueObject or ActiveRecord. Why is it impossible to implement those two without allowing direct access to members? Other than allowing explicit assignment such as:

Code: Select all

$obj->name = $name;
As opposed to:

Code: Select all

$obj->setName($name);
What is the difference?

Working with COM, CORBA, etc (as far as I understand) it is actually impossible to explose members directly. Even if a language has constructs for setting a member variable directly as a property, such as VB. There is some lower level construct which eventuallu invokes a member function to actually deal with setting/validating a member variable.

Perhaps I have the wrong impression, but is this not one of the purposes behind PHP5 __get/__set functions? They act something of a catch-all for manually get/set member variables. To me, tha suggests it's consider best practice to avoid directly setting member variables and favour methods instead.

Cheers :)
User avatar
stereofrog
Forum Contributor
Posts: 386
Joined: Mon Dec 04, 2006 6:10 am

Re: My First Class

Post by stereofrog »

psurrena wrote: This class is for querying the db and returning result:
This is exactly the problem. Classes are not "for" something, classes ARE something, in other words, concentrate on what class IS, not on what it DOES.

There is a very simple technique for the OO design: write down what you want to do in plain english, then underline all nouns and verbs in your description. Nouns become classes and verbs become methods.

So, how do we work with a database? We open a connection, execute queries, fetch data from results and optionally close the connection. In php, this can look like this

Code: Select all

class DbConnection 
{
   function open($credentials)
   function close()
}

class DbQuery 
{
   function execute(DbConnection $conn, $sqlStatement) ... returns DbResult
}

class DbResult 
{
   function fetchRow() ... returns array
}
Try implementing an API like this. Might be a good exercise.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Re: My First Class

Post by superdezign »

stereofrog wrote:Try implementing an API like this. Might be a good exercise.
I haven't seen a querying class separate from the connector in that fashion before, where the query function needs a reference to the connection on every call. If you follow this structure, maybe sending the connection through the constructor of the query class would be a better approach than through the querying function.
User avatar
stereofrog
Forum Contributor
Posts: 386
Joined: Mon Dec 04, 2006 6:10 am

Post by stereofrog »

Perhaps more common is to have a factory method (like createStatement) in the Connection. I tried to keep things as transparent as possible though.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

stereofrog wrote:Perhaps more common is to have a factory method (like createStatement) in the Connection. I tried to keep things as transparent as possible though.
Oh, to allow different types of connections? I could see that.
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

Post by psurrena »

Would everyone say that: viewtopic.php?t=68300&highlight=dbconnect is a good model to start with? To be honest a lot of the discussion in this thread is a bit over my head at this point.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

Hockey wrote:Perhaps I have the wrong impression, but is this not one of the purposes behind PHP5 __get/__set functions? They act something of a catch-all for manually get/set member variables. To me, tha suggests it's consider best practice to avoid directly setting member variables and favour methods instead.
One of the reasons for __get/__set is that they allow the cleaner syntax of using/setting properties directly, yet still being able to wrap the assignment in code as appropriate.
(#10850)
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

psurrena wrote:Would everyone say that: viewtopic.php?t=68300&highlight=dbconnect is a good model to start with? To be honest a lot of the discussion in this thread is a bit over my head at this point.
Hehe, I remember that one. That thread drove me to rewrite parts of my database wrapper, but it basically boils down to the format the stereofrog wrote up. That discussion is a very good on though, as it's the only one I've seen that really goes into detail as to what the database class should be responsible for and what it shouldn't.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

psurrena wrote:Would everyone say that: viewtopic.php?t=68300&highlight=dbconnect is a good model to start with? To be honest a lot of the discussion in this thread is a bit over my head at this point.
Well I recommended that thread so obviously I do. I thin scottayy eventually got to a very basic version here:

viewtopic.php?p=384959#384959

It's not perfect, but workable. I would recommend rereading the thread and jotting down what was "over your head." I think with a little clarification it will seem pretty straightforward to you.

Here is a tweak of scottayy's code:

Code: Select all

<?php

class Db_Connect_Mysql
{
    private $_host;
    private $_username;
    private $_password;
    private $_link;
    private $_errmsg = '';
   
    public function __construct($host, $username, $password)
    {
        $this->_host = $host;
        $this->_username = $username;
        $this->_password = $password;
    }
   
    public function connect()
    {
        $this->_link = mysql_connect($this->_host, $this->_username, $this->_password);
    }
   
    public function useDb($db)
    {
        mysql_select_db($db, $this->_link);
    }
   
    public function escape($var)
    {
        return mysql_real_escape_string($var, $this->_link);
    }
   
    public function query($sql)
    {
        if (!$this->_linkId) {
            $this->connect();
        }
       
        $result = mysql_query($sql, $this->_link);
        return new Db_Result_Mysql($this->_link, $result);
    }

    public function error()
    {
        return mysql_error($this->_link);
    }

}


class Db_Result_Mysql
{
    private $_link;
    private $_result;
   
    public function __construct($link, $result)
    {
        $this->_link = $link;
        $this->_result = $result;
    }
   
    public function numRows()
    {
        return mysql_num_rows($this->_result);
    }
   
    public function fetchRow($fetch_type=MYSQL_ASSOC)
    {
        return mysql_fetch_array($this->_result, $fetch_type);
    }

    public function error()
    {
        return mysql_error($this->_link);
    }
}
Maybe a generic Table Data Gateway style interface:

Code: Select all

$db = new Db_Connect_Mysql('localhost', 'foo', 'bar');
        $people=new PeopleGateway($db);
        $row = $people->find($id);
        $rows = $people->findByJob('programmer');
(#10850)
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

Post by psurrena »

Thank you everyone for all your help!
Post Reply