Page 1 of 1
My First Class
Posted: Tue Aug 14, 2007 12:53 pm
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'];
?>
Posted: Tue Aug 14, 2007 1:04 pm
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

Posted: Tue Aug 14, 2007 1:11 pm
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.
Posted: Tue Aug 14, 2007 1:13 pm
by psurrena
Oh! And thanks for fixing the loop. Why does $arr not require $this->?
Posted: Tue Aug 14, 2007 1:31 pm
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.
Posted: Tue Aug 14, 2007 2:18 pm
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:
As opposed to:
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

Re: My First Class
Posted: Tue Aug 14, 2007 2:24 pm
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.
Re: My First Class
Posted: Tue Aug 14, 2007 2:44 pm
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.
Posted: Tue Aug 14, 2007 3:04 pm
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.
Posted: Tue Aug 14, 2007 3:08 pm
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.
Posted: Tue Aug 14, 2007 3:53 pm
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.
Posted: Tue Aug 14, 2007 3:54 pm
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.
Posted: Tue Aug 14, 2007 4:10 pm
by superdezign
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.
Posted: Tue Aug 14, 2007 6:19 pm
by Christopher
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');
Posted: Tue Aug 14, 2007 8:29 pm
by psurrena
Thank you everyone for all your help!