Posted by Carl on 11/15/30 11:27

news@celticbear.com wrote:
> Well, I wrote my first PHP class today. Yeah!
> But to get it to work, in each function within the class I have to
> repeat the database connection lines, and that just seems redundant;
> there has to be a better way that I'm just not bright enough to think
> of.
>
> Any suggestions?
> (It's the first 3 lines of each of the two functions below. When I have
> it fully written, there will be about 10 similar functions, each
> repeating those three lines.)
>
> Thanks for any feedback!
> Liam
>
> class e_test {
>
> function gettech() {
> include('../Connections/userDBconnect.php');
> $db = mysql_select_db("printing", $connection) or $error .= "Could
> not select database1: " . mysql_error()."<p />";
> $dbconn = $connection;
> // CREATE TECH ARRAY
> $sql_T = "SELECT email FROM tbl_users WHERE dept = '1' AND
> active='1'";
> $result_T = @mysql_query($sql_T, $dbconn) or $error .= "Could not
> query1: " . mysql_error()."<p />";
> $count_T = mysql_num_rows($result_T) or $error .= "Could not select
> query2: " . mysql_error()."<p />";
> $i_T = 0;
> while ($row_T = mysql_fetch_array($result_T)) {
> $email_T = $row_T[email];
> $techs .= $email_T;
> $i_T++;
> if ($i_T < $count_T) {
> $techs .= ",";
> }
> }
> echo $error;
> return $techs;
> }
>
> function getdez() {
> include('../Connections/userDBconnect.php');
> $db = mysql_select_db("printing", $connection) or $error .= "Could
> not select database2: " . mysql_error()."<p />";
> $dbconn = $connection;
> // CREATE DESIGNER ARRAY
> $sql_D = "SELECT email FROM tbl_users WHERE dept = '3' AND
> active='1'";
> $result_D = @mysql_query($sql_D, $dbconn) or $error .= "Could not
> query3: " . mysql_error()."<p />";
> $count_D = mysql_num_rows($result_D) or $error .= "Could not query4:
> " . mysql_error()."<p />";
> $i_D = 0;
> while ($row_D = mysql_fetch_array($result_D)) {
> $email_D = $row_D[email];
> $designers .= $email_D;
> $i_D++;
> if ($i_D < $count_D) {
> $designers .= ",";
> }
> }
> echo $error;
> return $designers;
> }
>
> function getall() {
> $this->group = $techs.",".$designers;
> $thelist = $this->gettech().",".$this->getdez();
> return $thelist;
> }
>
> }
>
> $maillist = new e_test();
> echo $maillist->gettech();
> echo "<p />";
> echo $maillist->getdez();
> echo "<p />";
> echo $maillist->getall();
>

My suggestions follow.

- create another method that does all the hard work of selecting the
email addresses with one parm which determines the department id you
want to select, or blank for all dept's. (semi-pseudo code, will not
compile)

getEmailAddresses($departmentId='DEPT_ALL') {
$sql = "SELECT email FROM tbl_users WHERE active='1'";
if ($departmentId != 'DEPT_ALL') {
$sql .= ' AND dept = ' . $departmentId ;
}

$addresses = array();

$result = runQuery($sql)
while ( $result->hasMoreRows() ){
$addresses[] = /* get e-addr from $result */
}

return $addresses;
}

-- from within your gettech() and getDez() methods:
function getTech(){
return getEmailAddresses('1');
}

Note however that although you did indeed create a class, this class
functions more like a collection of functions than an object. I find
that a good place to start when designing classes/objects is to assign
it an object name, something that describes what it is in basic terms.
Using this name you can then get an idea of the behavior (methods)
expected of this object and the attributes (variable) that the object
must have.

You can also clean up the code quite a bit by moving the database
specific code into the userDBconnect code. Ideally, this class should
know as little about the database implementation as possible..

Hope this helps.
Carl.

[Back to original message]