Page 1 of 1

code inspection...

Posted: Fri Apr 27, 2007 4:34 am
by codeblazer
Sorry,

I need opinion on this code I have written; for example I am uncertain about things like:
1) is it scalable?
2) are there better ways of "analyzing" the problem and "designing" the solution for my problem domain?
3) the decision to require a class to depend on an ADBODB object - is this a bad choice?
4) am i doing enough checks for integrity and state? (checking vars, catching possible invalidities, etc)
5) OR am i totally going about this whole programming thing the wrong way?

Problem domain:
I intend on creating websites where at the minimum a website will have members; a member is described by the class below. In some cases the website can also have a business member which always will extend the Member class.

I want code that is strictly written for my set of requirements - not written as a generalisation. This way I know my code when my site goes live - and it is hopefully as efficient as it can be.

These are my goals - robust and efficient code. Please help me out! thanks...



Code: Select all

_Exceptions.inc.php
<?php

	class DB_trans_not_executed_adodb5 extends Exception
	{
		public function __construct()
		{
			print "dberror - if your browser doesnt support javascript please click <a create href=''>here</a><br>";
		}
	}

?>

_Member.php
<?php
include "_Exceptions.php.inc";


//all database transactions are tried first - class user may hold on to class details
//and refresh/get-new data by loading, during idle time database may lose connection
class Member
{
		private $id;
		private $email_address;
		private $username;
		private $password;
		private $date_join;
		private $dbh;//adodb5 handler

		//-1 id indicates a non-registered member instance
		public function __construct($dbh='NULL', $id='-1')
		{
			$this->id = $id;
			$this->dbh = $dbh;

			//type check for adodb object
			if(preg_match("/^ADODB_/", get_class($this->dbh)))
			{
				Member::load_member($this->id);
			}
			else
			{
				echo "Error occured: 0001 - you have loaded a member with a non-adodb object (_Member.php)";
				die();
			}
		}//constructor


		//method can be used to update with different Member attributes
		public function load_member($id='-1')
		{
			if((int)$id > 0 && is_integer((int)$id) )
			{
				$sql = "select * from member where id=? limit 1";
				try
				{
					$this->dbh->Prepare($sql);
					$row = $this->dbh->GetRow($sql, $id);
					if(!is_array($row))
					{
						throw new DB_trans_not_executed_adodb5;
					}
					else
					{
						$this->id = $row['id'];
						$this->email_address = $row['email_address'];
						$this->username = $row['username'];
						$this->password = $row['password'];
						$this->date_join = $row['date_join'];
					}
				}
				catch (DB_trans_not_executed_adodb5 $e)
				{
					echo"Redirecting";//redirect here...
				}
			}
			else
			{
			}
		}//load_member

		public function return_data($attrb)
		{
			return $this->$attrb;
		}//return_data_array

}



class MemberUse extends Member
{
	private $auth;
	private $member_type;//later incorporate member levels - admin etc.

	public function __construct($email_address='', $username='', $password='')
	{
		$this->email_address = $email_address;
		$this->username = md5($username);
		$this->password = $password;
		MemberUse::set_auth();
	}//constructor

	private function set_auth()
	{
		$sql = "select id, email_address, password, date_join from member where username = {?} limit 1";
		$stmt = $this->dbh->Prepare($sql);
		$row = $this->dbh->GetRow($stmt, array($this->username));

		if (is_array($row))
		{
			if ($row['password'] != $this->password)
			{
				$auth = "bad";
			}
			else
			{
				$auth = "good";
			}
		}
		else
		{
			$auth = "new";
		}
	}

	public function insert_member()
	{
		if((int)$this->id == -1)//confirm no id has been given to constructor, or set
		{
			$vld_rslt = Member::validate_self_data(); //validate result
			if (sizeof($vld_rslt) > 0)//validate function returns an array with field=>valid=>reason array
			{
				print_r($vld_rslt);//later you may wish to return this to caller for display etc
			}
			else
			{
				$sql = "insert into member (email_address, username, password, date_join) values (?,?,?,?)";
				$stmt = $this->dbh->Prepare($sql);
				$this->dbh->Execute($stmt,array($this->email_address, $this->username, md5($this->password), '101010101'));
				return true;
			}
		}
		else//for some reason insert member has been called with an id that may exist
		{
			echo "bad"; //throw new exception here
		}
	}//insert_member


	public function update_email_address($email_address='')
	{
		if($this->email_address != $email_address)
		{
			$rslt = array();
			$email_exists_id = Member::does_email_exist($email_address);
			//do some basic validation - format and duplicate (ignoring those of same id)
			if(Member::check_email($email_address) === false)
			{
				array_push($rslt, array ("email"  => array("valid" => "false", "reason" => "bad_format")));
				return $rslt;
			}
			else if (is_integer($email_exists_id) && $email_exists_id != $this->id)
			{
				array_push($rslt, array ("email"  => array("valid" => "false", "reason" => "duplicate")));
				return $rslt;
			}
			else
			{
				$sql = "update member set email_address = ? where id = '{$this->id}'";
				$stmt = $this->dbh->Prepare($sql);
				$this->dbh->Execute($stmt,array($email_address));
				$this->email_address = $email_address;
				return true;
			}
		}
		else
		{
			return false;
		}
	}//update_email_address


	public function update_password($password='')
	{
		$rslt = array();

		if ($this->password != md5($password))
		{
			if(Member::check_password($password) === false)
			{
				array_push($rslt, array ("password"  => array("valid" => "false", "reason" => "bad_format")));
				return $rslt;
			}
			else
			{
				$sql = "update member set password = ? where id = '{$this->id}'";
				$stmt = $this->dbh->Prepare($sql);
				$this->dbh->Execute($stmt,array(md5($password)));
				$this->password = $password;
				return true;
			}
		}
		else
		{
			return false;
		}
	}//update_password

//***************************************PRIVATE METHODS START*************************************



	private function validate_self_data()
	{
		$rslt = array();

		if(Member::check_username($this->username) === false)
		{
			array_push($rslt, array ("username"  => array("valid" => "false", "reason" => "bad_format")));
		}
		else if ((int)Member::does_username_exist($this->username) > 0)
		{
			array_push($rslt, array ("username"  => array("valid" => "false", "reason" => "duplicate")));
		}

		if(Member::check_email($this->email_address) === false)
		{
			array_push($rslt, array ("email"  => array("valid" => "false", "reason" => "bad_format")));
		}
		else if ((int)Member::does_email_exist($this->email_address) > 0)
		{
			array_push($rslt, array ("email"  => array("valid" => "false", "reason" => "duplicate")));
		}

		if(Member::check_password($this->password) === false)
		{
			array_push($rslt, array ("password"  => array("valid" => "false", "reason" => "bad_format")));
		}

		return $rslt;
	}//validate_self_data


	private function does_username_exist($user='')
	{
		$sql = "select id from member where username=? LIMIT 1";
		$this->dbh->Prepare($sql);
		$row = $this->dbh->Execute($sql, $user);

		if ((int)$row->RecordCount() > 0)
		{
			$row = $this->dbh->GetRow($sql, $user);
			return $row['id'];
		}
		else
		{
			return false;
		}
	}//does_username_exist

	private function does_email_exist($email='')
	{
		$sql = "select id from member where email_address=? LIMIT 1";
		$this->dbh->Prepare($sql);
		$row = $this->dbh->Execute($sql, $email);

		if ((int)$row->RecordCount() > 0)
		{
			$row = $this->dbh->GetRow($sql, $email);
			return $row['id'];
		}
		else
		{
			return false;
		}
	}//does_email_exist

	private function check_username($user='')
	{//later can be modified to incorporate mandatory alpha-numeric usernames etc.
		if(strlen($user) < 
		{
			return false;
		}
		else
		{
			return true;
		}
	}//check_username

	private function check_email($email='')
	{
		$email = trim($email);
		if(preg_match("/([\w\-]+\@[\w\-]+\.[\w\-]+)/",$email))
		{
			return true;
		}
		else
		{
			return false;
		}
	}//check_email

	private function check_password($pass='')
	{//later can be modified to incorporate mandatory alpha-numeric passwords etc.
		if(strlen($pass) < 
		{
			return false;
		}
		else
		{
			return true;
		}
	}//check_password

}


?>

Posted: Fri Apr 27, 2007 5:48 am
by Chris Corbyn
:arrow: Moved to Coding Critique