Jump to content

Learning OOP - Need pointer in the right direction.


iPixel

Recommended Posts

So i've been wanting to expand my php horizons if you will and finally break of my fear of OOP.

Here's a simple connection script i built, however i'm kind of stuck on a part of it.

 

user passes username/password.

 

i instantiate the mysqlcon class and call the connect function.

the connect function attempts a connection to the database.

if successful, pass on the username and password to the authenticator class.

call the authenticate function.

select user based on username/passwrod criteria.

if found return "Confirmed" otherwise "Denied"

 

here's my issue, i'm assuming that my

return $this->ok

variable is returned back to the mysqlcon class. however how can i return that variable once more back outside of the object class so that i can continue with my script assuming authentication was confirmed.

 

Here's my code... thanks for the help!

 

<?php
## OOP PRACTICE ##

class mysqlcon
{
	function connect($user,$pass)
		{
			//Note: Connect to the database
			$db = mysql_connect("localhost", "usr","psw") or die(mysql_error()); 
			if(mysql_select_db("database",$db) or die(mysql_error()))
				{
					$auth = new authenticator();
					$auth->authenticate($user,$pass);
				}

		}
}

class authenticator
{
	var $ok = "Confirmed";
	var $ex = "Denied";
	function authenticate($user,$pass)
		{
			//Note: User user and pass variables to check for user in table
			$qry = "SELECT * FROM users WHERE username = '$user' AND password = '$pass'";
			$sql = mysql_query($qry) or die(mysql_error());
			$cnt = mysql_num_rows($sql);
			if($cnt > 0)
				{
					return $this->ok;
				}
			else
				{
					return $this->ex;
				}
		}
}



$con = new mysqlcon();
$con->connect($_POST['username'],$_POST['password']);

?>

 

PS: the classes are in class.php and the instantiation are in index.php. I just placed them together for easy of viewing.

Link to comment
Share on other sites

I got it working, but any criticism or enhancements to my code would be most appreciated.

 

<?php
## OOP PRACTICE ##

class mysqlcon
{
	function connect($user,$pass)
		{
			//Note: Connect to the database
			$db = mysql_connect("localhost", "usr","psw") or die(mysql_error()); 
			if(mysql_select_db("monet3",$db) or die(mysql_error()))
				{
					$auth = new authenticator();
					return $auth->authenticate($user,$pass);

				}

		}
}

class authenticator
{
	var $ok = "Confirmed";
	var $ex = "Denied";
	function authenticate($user,$pass)
		{
			//Note: Use the user and pass variables to check for user in table
			$qry = "SELECT * FROM users WHERE username = '$user' AND password = '$pass'";
			$sql = mysql_query($qry) or die(mysql_error());
			$cnt = mysql_num_rows($sql);
			if($cnt > 0)
				{
					return $this->ok;
				}
			else
				{
					return $this->ex;
				}
		}
}



$con = new mysqlcon();
if($con->connect("$_POST['username']","$_POST['password']") == "Confirmed")
{
	echo "User Authenticated!";
}

?>

Link to comment
Share on other sites

I think you have the relationship backwards.  Your authenticator should use/contain your db object, not the other way around.  Why?  Because a db object shouldn't care about authentication.  It has a singular purpose - execute db queries and return their results and other metadata about the queries (like number of rows affected).  The authenticator should handle the higher level stuff.

 

I'd also make your authenticate method a pure boolean.  Get rid of $ex/$ok - they're superfluous - and simply return true or false.  Rename your method to isAuthenticated, and you can do something like:

 

if($auth->isAuthenticated($user, $pass))
{
   echo "Welcome!";
}
else
{
   // handle it
}

 

Better yet, if you don't need the authenticator to maintain state, turn the function static and call it like:

 

if(Authenticator::isAuthenticated($user, $pass))
{
   // etc.
}

 

For more of a general idea of how to determine good code, Google the following:

 

Separation of Concerns

Single Responsibility Principle

DRY - Don't Repeat Yourself

Tell, Don't Ask

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.