Does this look right?

[baggyn]

Hey

i've just been reading your PHP OOP tutorials and i think there very informative so i decided to give OOP a go.

i decided to try and do a simple login system which i know i can easily achieve in procedural code. I tried to follow the principles mentioned in 'OO PHP Part 2: Boring OO Principles' and this is what i came up with. is this vaguely right or have i completely missed the point?

 

login.php

<?php
include('includes/connect.php');
include('includes/datamap.php');


$login = new LoginDataMap;
$pass = $login->check_login($_POST['username'],sha1($_POST['password']));
if($pass == true)
{
	header("Location: main.php");
}
else
{
	header("Location: index.php?ErrorId=1");
}


?>

 

 

connect.php

<?php
class connect
{
	private $host;
	private $db_name;
	private $password;

	function __construct()
	{
		$this->host = 'localhost';
		$this->db_name = "db_name";
		$this->password = "password";
	}

	function make_connection()
	{
		mysql_connect($this->host,$this->db_name,$this->password);
		mysql_select_db($this->db_name);
	}

}

$con = new connect();
$con->make_connection();

?>

 

datamap.php

<?php
abstract class DataMapper
{	
	function execute_query($query)
	{
		return mysql_query($query);
	}
	function fetch_num_rows($rs)
	{
		return mysql_num_rows($rs);
	}

}

class LoginDataMap extends DataMapper
{	
	public function check_login($username,$password)
	{
		$num = parent::fetch_num_rows(parent::execute_query("SELECT * FROM `users` WHERE `username` = \"$username\" AND `password`= \"$password\""));
		if($num == 1)
		{
			return true;
		}
		else
		{
			return false;
		}
	}

}
?>

[gizmola]

Looks good. Currently there's not really a reason not to have your check_login be declared static, because you are really just using it statically.  As for the basic concepts you've used quite a few, with the abstract class etc.  It's simple enough but what you've done is clean and concise.

[baggyn]

thanks for your input 

[448191]

I know this is a fairly old topic, but as the author of the tutorials in question, I feel I should comment on this (better use the Tutorial board next time).

 

The 'OO principles' tutorial doesn't describe the Data Mapper pattern. It merely uses it as a quick example. Take another look at section 2.2 though. You'll notice that what is called 'Domain Logic' (the login operation) is encapsulated in the User object, not in the Data Mapper. A Data Mapper is used solely to 'map' objects' data to the corresponding table row(s).

 

Class connect is procedural. Usually, when your class name equals an operation, your doing something wrong, with the exception of Command Objects, but these most commonly represent 'user' operations.

 

As I said, Data Mapper is used solely to map data (it's responsibilty). Thus, execute_query() and fetch_num_rows() do not belong in the Data Mapper hierarchy.

 

Now, take those two methods, move them to your 'connect' class, rename it 'DatabaseAdapter' or something else that indicates that it is used for providing an interface to the database, and things start looking a lot better. Pass an instance of DbAdapter to a DataMapper (THAT you can put in the abstract class), move the connection making to the Transaction Script (login.php) or in a file to include (hey, new found use for connect.php), eh voila, even better.

 

Good luck.

 

Edit: moved to Tutorial Help for reference