Two Questions

[Lamez]

1.OpenID, if I add it to my website, will I be able to gather data such as email, first name, last name, etc.

 

2.I am leaning OOP in PHP and I am trying to make a mysqli class, I'm not very good at it. Maybe you guys can help me here is what I have so far. Am I doing okay?

 

<?php
class database{
	var $mysqli;
	function __construct($host, $user, $pass, $db){
		$this->mysqli = new MySQLi($host, $user, $pass, $db);
	}
	function query($query){
		return mysqli_query($this->mysqli, $query);
	}
	function fetchAssoc($query){
		return mysqli_fetch_assoc(mysqli_query($this->mysqli, $query));
	}
	function numRows($query){
		return mysqli_num_rows(mysqli_query($this->mysqli, $query));
	}
	function delete($table, $where){
		return $this->query("DELETE FROM $table WHERE ".$where);
	}
}
?>

[ignace]

1.OpenID, if I add it to my website, will I be able to gather data such as email, first name, last name, etc.

 

Depends if you mean from OpenID then no. If you mean through forms etc. then yes. OpenID provides an API for authentication, it only tells you the user successfully logged in.

 

2.I am leaning OOP in PHP and I am trying to make a mysqli class, I'm not very good at it. Maybe you guys can help me here is what I have so far. Am I doing okay?

 

Depends if you mean, did you create valid OOP then almost. If you mean how you build up your class then no. Don't just create a class to wrap another. Your class should add functionality not copy.

 

class My_MySQL {
  /*snip*/
  public function fetchRow($mode = MYSQLI_ASSOC) {
    return mysqli_fetch_array($this->result, $mode);
  }
  public function fetchAll($mode = MYSQLI_ASSOC) {
    $rows = array();
    while ($row = $this->fetchRow($mode))
      $rows[] = $row;
    return $rows;
  }
  /*snip*/
}

[Lamez]

Okay Thanks for the help.

 

So do I not need to connect? Or are you just showing me an example? I am a bit new to mysqli as well.

[ignace]

Okay Thanks for the help.

 

So do I not need to connect? Or are you just showing me an example? I am a bit new to mysqli as well.

 

Of course you need to connect all I am saying is that you should not create a class to merely wrap another, add functionality don't copy.

[Lamez]

Okay gotcha, sorry I was tired when I was responding. I just woke up. I will finish writing my class and post it here for inspection.

[Lamez]

Well I don't think my fetchAll function working. I am getting an error as such: "Maximum execution time of 60 seconds exceeded"

 

Maybe I am doing something wrong? Here is my entire class:

 

<?php
class database{
	var $mysqli;
	function __construct($host, $user, $pass, $db){
		$this->mysqli = new MySQLi($host, $user, $pass, $db);
	}
	function query($query){
		return mysqli_query($this->mysqli, $query);
	}
	function fetchAssoc($query){
		return mysqli_fetch_assoc($this->query($query));
	}
	function fetchAll($query){
		$rows = array();
		while($f = $this->fetchAssoc($query))
			$rows[] = $f;
		return $rows;
	}
	function numRows($query){
		return mysqli_num_rows($this->query($query));
	}
	function delete($table, $where){
		return $this->query("DELETE FROM $table WHERE ".$where);
	}
}
?>

 

Here is how I am using it:

$db = new database(DB_HOST, DB_USER, DB_PASS, DB_DB);
print_r($db->fetchAll("SELECT * FROM people WHERE id != '-1'"));

 

Any thoughts?

[ignace]

Maybe I am doing something wrong?

 

Yup, you are passing the query to fetchAssoc() which executes the query and then returns the first result set (all the time):

 

mysql_fetch_assoc(mysql_query($query));

 

I'm not sure that you know what this means but your query is executed on each loop which means that you will always get the first result back so $rows is populated with the first row (a few thousand times).

 

These kind of error's you should be able to pick up as a programmer as they are very hard to track and your parser does not return any syntax error's, as they are so called logical error's.

[ignace]

Try:

 

class Database {
  private $connection = null;
  private $result = null;
  private $lastQuery = '';
  private $affectedRows = 0;
  
  public function __construct($name, $password, $username = 'root', $host = 'localhost') {
    $this->connection = new MySQLi($host, $username, $password, $name);
  }
  
  public function execute($query, $mode = MYSQLI_STORE_RESULT) {
    $this->lastQuery = $query;
    $result = $this->connection->query($query, $mode);
    if (is_object($result) && $result instanceof MySQLi_Result) {
      $this->result = $result;
      $this->affectedRows = $this->result->num_rows();
    } else {
      $this->affectedRows = $this->connection->affected_rows();
    }
    return $this;
  }
  
  public function fetchRow($mode = MYSQLI_ASSOC) {
    return $this->result->fetch_array($mode);
  }
  
  public function fetchAll($mode = MYSQLI_ASSOC) {
    return $this->result->fetch_all($mode);
  }
  
  public function getAffectedRows() {
    return $this->affectedRows;
  }
}

 

$db = new Database('name', 'password');
print_r($db->execute('SELECT * FROM people')->fetchAll());

[Andy-H]

Well I don't think my fetchAll function working. I am getting an error as such: "Maximum execution time of 60 seconds exceeded"

 

Maybe I am doing something wrong? Here is my entire class:

 

<?php
   class database{
      var $mysqli;
      function __construct($host, $user, $pass, $db){
         $this->mysqli = new MySQLi($host, $user, $pass, $db);
      }
      function query($query){
         return mysqli_query($this->mysqli, $query);
      }
      function fetchAssoc($query){
         return mysqli_fetch_assoc($this->query($query));
      }
      function fetchAll($query){
         $rows = array();
         while($f = $this->fetchAssoc($query))
            $rows[] = $f;
         return $rows;
      }
      function numRows($query){
         return mysqli_num_rows($this->query($query));
      }
      function delete($table, $where){
         return $this->query("DELETE FROM $table WHERE ".$where);
      }
   }
?>

 

Here is how I am using it:

$db = new database(DB_HOST, DB_USER, DB_PASS, DB_DB);
print_r($db->fetchAll("SELECT * FROM people WHERE id != '-1'"));

 

Any thoughts?

 

You were using fetchAll, however you were passing the query string rather than the result resource retuned from a mysqli_query call.

[Lamez]

You guys are helping me out a lot! Here is what I now have.

<?php
class Database{
	var $mysqli, $result, $q;
	function __construct($host, $user, $pass, $db){
		$this->mysqli = new MySQLi($host, $user, $pass, $db);
	}
	function execute($query){
		$this->q = $query;
		$this->result = $this->mysqli->query($query);
		return $this;
	}
	function fetchRow(){
		return $this->result->fetch_assoc();
	}
	function fetchAll(){
		return $this->result->fetch_all();
	}
}
?>

 

I'm not too sure if I am getting the corrected results. This is returned when using fetch_all():

Array ( [0] => Array ( [0] => 3 ) )

 

Should I be getting an array within an array?

[ignace]

Should I be getting an array within an array?

 

Yes, as each result is added to an array.

[Lamez]

Glad to hear I am headed down the right path. What do you make of my additions?

<?php
class Database{
	var $mysqli, $result, $q;
	function __construct($host, $user, $pass, $db){
		$this->mysqli = new MySQLi($host, $user, $pass, $db);
	}
	function execute($query, $mode = MYSQLI_STORE_RESULT){
		$this->q = $query;
		$this->result = $this->mysqli->query($query, $mode);
		return $this;
	}
	function fetchRow($mode = MYSQLI_ASSOC){
		return $this->result->fetch_assoc($mode);
	}
	function fetchAll($mode = MYSQLI_ASSOC){
		return $this->result->fetch_all($mode);
	}
	function numRows(){
		return $this->result->num_rows;
	}
	function delete($table, $where){
		return $this->execute("DELETE FROM ".$table." WHERE ".$where);
	}
	function deleteAll($table){
		return $this->execute("TRUNCATE ".$table);
	}
	function update($table, $set, $where){
		return $this->execute("UPDATE ".$table." SET ".$set." WHERE ".$where);
	}
	function insert($table, $values, $data){
		$valString = $dataString = "";
		$length = count($values);
		for($i = 0; $i<$length; $i++){
			$valString .= $values[$i];
			$dataString .= $values[$i]." = '".$data[$i]."'";
			if($i < $length-1){
				$valString .= ", ";
				$dataString .= ", ";
			}
		}
		return $this->execute("INSERT INTO ".$table." (".$valString.") VALUES (".$dataString.")");
	}
}
?>

[ignace]

In my version I accounted for the boolean that query returns when you execute something else then a SELECT query and I also accounted for a uniform approach to affected_rows and num_rows, in either case you would call getAffectedRows() and get the correct number of rows selected, inserted, updated, or deleted.

 

$db->execute('INSERT INTO table () VALUES ()')->numRows();

 

Would result in an error along the lines of "Calling method on a non-object ..." as where:

 

$db->execute('INSERT INTO table () VALUES ()')->getAffectedRows();

 

Wouldn't.

[Lamez]

I don't understand why affected rows is significant. Please explain why I need it? Oh, are you talking about when calling num rows?

[ignace]

I don't understand why affected rows is significant. Please explain why I need it?

 

On each of these occurrences you return the Database object.

 

return $this->execute("DELETE FROM ".$table." WHERE ".$where);

 

What would be a logical step for anyone using your database after executing a INSERT/UPDATE/DELETE query?

 

That's right. To check if anything was INSERTED/UPDATED/DELETED and how would they do that? Knowing the execute() function returns a reference to itself.

 

Also correct, you take a look at the plethora of functions the object provides, in this case numRows() would make sense. However if you call it, it returns "Calling an method on a non-object ..".

 

Don't just copy-paste code and remove as you see fit, as the code may have value. Learn to understand the code before you recklessly remove it, learn to understand the implications that it may have in doing so (like the before mentioned "Calling a method on a non-object").

 

I can't teach you, if you aren't willing to learn.

[Lamez]

Don't just copy-paste code and remove as you see fit, as the code may have value. Learn to understand the code before you recklessly remove it, learn to understand the implications that it may have in doing so (like the before mentioned "Calling a method on a non-object").

 

I can't teach you, if you aren't willing to learn.

 

This is a false statement in my case. I glanced at your posted code, but I did not copy it. It helped me understand OOP a little better. What really allowed me to fully understand how this should work is this: http://www.php.net/manual/en/mysqli.affected-rows.php Example #1.

 

After I had my code created, I compared it to yours and went back and added the modes.

 

I am very willing to learn, but I am not going to lie sometimes it's hard to understand you.

 

Let me see if I understand you now; Instead of returning the database object, return the affected rows?

[Lamez]

Wait, I completely understand now.

 

When the query is not a select query, num rows will suffice, but when using a select query, num rows will not work, but using your method of affected rows will work. That is why your execute method has that if statement in it?

[ignace]

When the query is not a select query, num rows will suffice

 

No, it's the other way around.

 

but using your method of affected rows will work. That is why your execute method has that if statement in it?

 

Yes, it's a uniform approach to the num_rows/affected_rows problem. Return $this is used for method chaining, like so:

 

$object->method()->method()->method()->method()->method()

 

The execute() method doesn't do much and we want to abstract away as much as possible without breaking the client code. For example:

 

foreach ($db->execute('SELECT * FROM table')->fetchAll() as $row) {
  print_r($row);
}

 

Of course this wouldn't work in my last posted code and you would need my previous code, where fetchAll() made use of fetchRow() or the code was changed to:

 

$rows = $this->result->fetch_all($mode);
return !empty($rows) ? $rows : array();

 

This would not break the client code as in the case where fetch_all() would return nothing an array would be returned.

[Lamez]

Awesome. I have my database class working well. I even wrote a small HTML class for easier use. Now I am working on a Email class based on pear's SMTP classes.

 

I have this:

<?php
require_once("pear/Mail.php");
class Email extends Database{
	var $from_name, $from_email, $to_name, $to_email, $subject, $body, $host, $port, $username, $password;
	function dbEmail($id, $name, $email, $subject, $body){
		$q = $this->select(TBL_SMTP, "*", "id = '".$id."'");
		$n = $q->numRows();
		if($n > 0){
			$f = $q->fetchRow();
			$this->from_name = $f['name'];
			$this->from_email = $f['email'];
			$this->username = $f['username'];
			$this->password = $f['password'];
			$this->host = $f['protocol']."://".$f['server'];
			$this->port = $f['port'];
			$this->to_name = $name;
			$this->to_email = $email;
			$this->subject = $subject;
			$this->body = $body;
			return sendEmail();
		}else
			return false;
	}
	function sendEmail(){
		$from = $this->from_name." <".$this->from_email.">";
		$to = $this->to_name." <".$this->to_email.">";
		$headers = array ('From' => $this->from,
  					  			'To' => $this->to,
  					  		'Subject' => $this->subject);		  
			$smtp = Mail::factory('smtp',
			array ( 'host' => $this->host,
    			   	'port' => $this->port,
    				'auth' => true,
    			'username' => $this->username,
    			'password' => $this->password));
		$mail = $smtp->send($to, $headers, $this->body);
		if(PEAR::isError($mail)){
  				//echo($mail->getMessage()); //For debugging purposes only
			return false;
			}else
  				return true;
	}
	function addAccount($name, $email, $username, $password, $protocol, $port, $server){
		//nothing yet..
	}
}
$email = new Email();
?>

I'm getting errors of such:

 

Warning: Missing argument 1 for Database::__construct(), called in C:\Wamp\www\KrazyPickem 2.0\core\includes\Email.php on line 47 and defined in C:\Wamp\www\KrazyPickem 2.0\core\includes\Database.php  on line 4

 

An so on..

 

I thought if it was a child class I did not have to create a new database:

 

$db = new Database(...);

 

How do I handle this situation?

 

Oh ya, just so you know I took your advice, and fixed my logical error:

//...
	function execute($query, $error = false, $mode = MYSQLI_STORE_RESULT){
		$this->q = $query;
		if(!$error)
			$result = $this->mysqli->query($query, $mode);
		else
			$result = $this->mysqli->query($query, $mode) or die($this->mysqli->error);

		if(is_object($result) && $result instanceof MySQLi_Result){//if result is a object and is part of the mysqli class?
			$this->result = $result;
			$this->affectedRows = $this->result->num_rows;
		}else
			$this->affectedRows = $this->mysqli->affected_rows;
		return $this;
	}
	function numRows(){
		return $this->affectedRows;
	}
//...

 

[ignace]

Your Email class violates the LSP (Liskov Substitution Principle) which states that every child should be substitutable for it's parent.

 

class UserService {
  private $db = null;
  
  public function __construct(Database $db) {
    $this->db = $db;
  }
  
  public function findUserByName($name) {
    return $this->db->execute("SELECT * FROM table WHERE name = '$name'")->fetchAll();
  }
}

 

Does the below make sense to you?

 

$userService = new UserService(new Email());

 

Another example what happens when I would want to load e-mails from a service, like an external CRM? Extends implies an is-a relationship, does this makes sense to you?

 

An e-mail is-a database

[Lamez]

Yes, email is-a database. My email class is also a database class, in a nutshell?

 

So I now have this:

<?php
require_once("pear/Mail.php");
class Email extends Database{
	var $from_name, $from_email, $to_name, $to_email, $subject, $body, $host, $port, $username, $password;
	private $db = NULL; //$db will is a object?
	function __construct(Database $db){
		$this->db = $db;
	}
	function dbEmail($id, $name, $email, $subject, $body){
		$q = $this->db->select(TBL_SMTP, "*", "id='".$id."'");
		if($q->numRows > 0){//I don't think this is correct.
			$f = $q->fetchRow();
			$this->from_name = $f['name'];
			$this->from_email = $f['email'];
			$this->username = $f['username'];
			$this->password = $f['password'];
			$this->host = $f['protocol']."://".$f['server'];
			$this->port = $f['port'];
			$this->to_name = $name;
			$this->to_email = $email;
			$this->subject = $subject;
			$this->body = $body;
			return sendEmail();
		}else
			return false;
	}
	function sendEmail(){
		$from = $this->from_name." <".$this->from_email.">";
		$to = $this->to_name." <".$this->to_email.">";
		$headers = array ('From' => $this->from,
  					  			'To' => $this->to,
  					  		'Subject' => $this->subject);		  
			$smtp = Mail::factory('smtp',
			array ( 'host' => $this->host,
    			   	'port' => $this->port,
    				'auth' => true,
    			'username' => $this->username,
    			'password' => $this->password));
		$mail = $smtp->send($to, $headers, $this->body);
		if(PEAR::isError($mail)){
  				//echo($mail->getMessage()); //For debugging purposes only
			return false;
			}else
  				return true;
	}
	function addAccount($name, $email, $username, $password, $protocol, $port, $server){
		//nothing yet..
	}
}
$db = new Database(DB_HOST, DB_USER, DB_PASS, DB_DB);
$email = new Email($db);
?>

 

I believe I am getting this down. Maybe?

[ignace]

Yes, email is-a database.

 

So every e-mail you send (with Gmail, Hotmail, Outlook, ..) is a database?

[Lamez]

Yes, email is-a database.

 

So every e-mail you send (with Gmail, Hotmail, Outlook, ..) is a database?

 

From what I understand. Yes (In this context of course). Class database is the parent, and email is the child. Every child is apart of their parent?

[ignace]

Yes (In this context of course)

 

This context you are referring to is an illusion. An e-mail is not a database, and never will be. Your application should always model after real-world objects. You don't add/update/delete records in an e-mail, do you? A correct implementation would be:

 

class Mail {
  private $from = '';
  private $to = array();
  private $headers = array();
  private $subject;
  private $message;
  
  public function setFrom($email, $name = null) {
    $this->from = $name ? "$name <$email>" : $email;
    $this->headers[] = 'Reply-To: ' . $this->from;
    $this->headers[] = 'Return-Path: ' . $this->from;
    return $this;
  }
  
  public function setSubject($subject) {
    $this->subject = $subject;
    return $this;
  }
  
  public function setMessage($message) {
    $this->message = str_replace("\n.", "\n..", $message);
    $this->message = wordwrap($this->message, 70);
    return $this;
  }
  
  public function addTo($email, $name = null) {
    $this->to[] = $name ? "$name <$email>" : $email;
    return $this;
  }
  
  public function send() {
    $to = implode(',', $this->to);
    $headers = implode("\r\n", $this->headers);
    return mail($to, $this->subject, $this->message, $headers);
  }
}

 

$users = $db->execute('SELECT concat(firstname, ' ', lastname) AS name, email_address FROM table')->fetchAll();
foreach ($users as $user) {
  $email->addTo($user['email_address'], $user['name']);
}

$email->send();