Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.
Maugrim_The_Reaper wrote:The way I think of objects are as small narrow-minded (maybe even miserly) packages of functionality.
I've written my first class about 1-2 months ago. It's a database class. The class connects to the database once it is called, then you can use one of the available methods: a method to run queries, a method which counts the number of rows inside a table and so on...
My question is, am I doing it wrong? Should I make a new class for each of these methods?
<?php
class db
{
private $class_member_1;
private $class_member_2;
.
.
.
private $class_member_n;
function __construct()
{
// Connect to the database
}
function __destruct()
{
// Close the connection
}
/* A simple function that should be used before inserting any data into the database */
public function esc($data)
{
if (get_magic_quotes_gpc())
{
$data = stripslashes($data);
}
$data = mysql_real_escape_string($data);
return $data;
}
/* rt_esc = runtime escape Strip slashes in case magic_quotes_runtime is set to 'On' */
public function rt_esc($data)
{
if (get_magic_quotes_runtime())
{
$data = stripslashes($data);
}
return $data;
}
/* Sends mysql query and returns the result, throws an error if the attempt fails */
public function query($query)
{
.
.
.
return $this->result;
}
/* Returns the number of records in a specific table
The second argument is optional but it is highly recommended to supply it
The second argument should be a column name - try to use an intger column (the smaller the better) */
public function count_rows($tbl, $col = '*')
{
return mysql_result(self::query(sprintf("SELECT COUNT('%s') FROM %s", $col, $tbl)), 0);
}
/* Returns a multidimensional associative array
Each element of the parent array holds one row from the database and each
one of these holds the columns of that particular row
If there is only one row the function returns a regular associative array */
public function fetch_assoc($query)
{
.
.
.
return $this->arr;
}
}
?>
private $db_info_file = 'db_info.php';
private $tries = 10; /* How deep should the class check for the db_info.php file */
private $link;
private $result;
private $row = array();
private $arr = array();
But as you probably noticed, I didn't post the whole code since the code itself is not the main point here. So I saved some space by cutting things off.
Your class looks like a pretty standard connection class. The two things that stand out to me are fetch_assoc() and $db_info_file = 'db_info.php'. The reason they do for me is that I think classes should to the minimum necessary to do a single thing well. In this case it is to maintain and support database connections. Usually there is a query() method that returns a Record Set or Active Record object. That object in turn is responsible for retrieving data from a successful query. Likewise $db_info_file implies that there is configuration file reading and management in the connection class, which is beyond what it should be responsible for doing. I would move the configuration stuff out to a separate Config class.
Last edited by Christopher on Thu Jun 08, 2006 5:11 pm, edited 1 time in total.
As for the "minimum necessary" thing - I totally agree and that's also why I've started learning OOP.
About the other class for the $db_info_file I'm not sure since all it does with this is:
/* Searching the file which holds the database info */
for (; $this->tries > 0; $this->tries--)
{
if (!file_exists($this->db_info_file))
{
/* If the file doesn't exist we go back one directory and so on until
we find it or until the max number of tries ($tries) has been reached */
$this->db_info_file = '../' . $this->db_info_file;
}
else
{
break;
}
}
if (!file_exists($this->db_info_file))
{
throw new exception('Cannot find the database\'s information file', 1);
}
require_once $this->db_info_file;
arborint wrote:I am hoping it doesn't set global vars!
Of course it doesn't...
arborint wrote:A nice simple configuration class would be preferable. It could also be used elsewhere.
It doesn't include a configuration file for some kind of application, only a file which holds the database's information.
I'm not going to include this file anywhere else, cause that would mean that I wish to connect to the database - and that's what the class is for
Oren wrote:I'm not going to include this file anywhere else, cause that would mean that I wish to connect to the database - and that's what the class is for
I usually keep configuration is a central place, then I can easily change settings for live, testing and development sites.
Since I mentioned the example which is a Strategy, I should also note a Factory Pattern should be looked for determining which strategy to actually implement in an app... Thought I'd mention it for reference.