10 Replies - 2410 Views - Last Post: 21 April 2012 - 12:00 PM Rate Topic: -----

#1 cxn  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 33
  • Joined: 10-March 12

Is this good practice? (OOP/PDO)

Posted 20 April 2012 - 04:51 AM

Hello all!

I'm trying to get into OOP/PDO using PHP.

The following method is to register a user:

public function createUser($user_name , $password, $is_admin = 0)
{	
	include('connect.php');	
	//Generate users salt
	$user_salt = $this->randomString();
			
	//Salt and Hash the password
	$password = $user_salt . $password;
	$password = $this->hashData($password);
			
	//Create verification code
	$code = $this->randomString();

	//Commit values to database here.

try {

$pdo = new PDO($dsn,$user,$pass); 
} catch(PDOException $e) {
die ('Error');
}

$sql = "INSERT INTO users (user_name, password) VALUES (:username,:password)";
$created = $pdo->prepare($sql);
$created->execute(array(':username'=>$user_name,
				  ':password'=>$password));
				  
				  
	if($created != false){
		return true;
	}
			
	return false;
}


At first it didn't work at all, I got an error on my pdo variable. After that I adeed 'include('connect.php');' and I added everything after '//Commit values to database here.'


This is my register.php page:

<?php 
require_once('classes/auth.php');
require_once('connect.php');

$auth = new Auth();

if(!isset($_POST['submit']))
{
	echo '<form action="' . htmlentities($_SERVER['PHP_SELF']) . '" method="post">
	<div><label for="username">Username:
	<input type="text" name="username" id="username" maxlength="20"/></label>
	</div>
	<div><label for="password">Password:
	<input type="password" name="password" id="password" maxlength="20"/></label></div>
	<div><input type="submit" name="submit" value="Register"/><input type="reset" value="Reset fields" /></div>
	</form>';
	
}
else
{	

$user_name = $_POST['username'];
$password = $_POST['password'];

$auth->createUser($user_name, $password);




    echo 'Thanks you for registering <b>' . $_POST['username'] . '</b>.<br />';
	echo 'Click <a href="login.php">here</a> to log in.<br />';

	
}
?>


I was wondering if all of this is a good way of coding. The initial classes/object code is taken from a tutorial here on DIC. I added the database stuff myself. I doubt whether the include('connect.php') is in its place...

Notice that the code is not fully being used (for example I do not store the salt yet - I first want this to work properly).

Any feedback/comments/hints are highly appreciated! Thanks

Is This A Good Question/Topic? 0
  • +

Replies To: Is this good practice? (OOP/PDO)

#2 Dormilich  Icon User is online

  • 痛覚残留
  • member icon

Reputation: 3394
  • View blog
  • Posts: 9,598
  • Joined: 08-June 10

Re: Is this good practice? (OOP/PDO)

Posted 20 April 2012 - 05:04 AM

there are issues.

- include/require do not belong inside a function/method, except when that method’s purpose is to include something (like an autoloader). if you need to include code required in a class, include it when you include the class (i.e. in the class definition file)

- PDO does not need to be instantiated in the method. better options are Dependency Injection or Service Locator (e.g. via Singleton)
// Dependency Injection
$auth = new Auth( new PDO(...) );
$auth->createUser(...);

class Auth
{
    private $pdo;

    public function __construct(PDO $pdo)
    {
        $this->pdo = $pdo;
        // ...
    }

    // ...

}


- I recommend to run PDO in Exception mode: $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);. this allows you to handle all DB errors sensibly and on a level where you can do something about the error (e.g. if the connection fails, skip all DB code)
Was This Post Helpful? 1
  • +
  • -

#3 cxn  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 33
  • Joined: 10-March 12

Re: Is this good practice? (OOP/PDO)

Posted 20 April 2012 - 05:23 AM

class Auth {
	private $_siteKey;
	
	public function __construct()
  	{
		$this->siteKey = 'my site key will go here';
	}


This is how my __construct function currently looks like, I think I need it for the rest to work out some how.

Where would I need to put this code?
// Dependency Injection
$auth = new Auth( new PDO(...) );
$auth->createUser(...);

class Auth
{
    private $pdo;

    public function __construct(PDO $pdo)
    {
        $this->pdo = $pdo;
        // ...
    }

    // ...

}



I only have one class file, auth.php where everything is in, also my functions.. I still don't fully understand OOP, thought I would try it with an example but doesn't make a lot of sense to me :(
Was This Post Helpful? 0
  • +
  • -

#4 CTphpnwb  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 2834
  • View blog
  • Posts: 9,740
  • Joined: 08-August 08

Re: Is this good practice? (OOP/PDO)

Posted 20 April 2012 - 06:35 AM

I don't think you need to do:
    public function __construct(PDO $pdo)


since $pdo is an object it will be passed by reference anyway:
    public function __construct($pdo)



Also, magic numbers are generally considered to be bad. HTML is just data anyway, so I'd read it in from an html file and modify it where needed.
Was This Post Helpful? 0
  • +
  • -

#5 Dormilich  Icon User is online

  • 痛覚残留
  • member icon

Reputation: 3394
  • View blog
  • Posts: 9,598
  • Joined: 08-June 10

Re: Is this good practice? (OOP/PDO)

Posted 20 April 2012 - 06:42 AM

View PostCTphpnwb, on 20 April 2012 - 03:35 PM, said:

I don't think you need to do:
    public function __construct(PDO $pdo)


since $pdo is an object it will be passed by reference anyway:
    public function __construct($pdo)


and how do you make sure $pdo is actually a PDO object? you could be passing anything there without Type Hinting. e.g. new Auth(1);
Was This Post Helpful? 0
  • +
  • -

#6 CTphpnwb  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 2834
  • View blog
  • Posts: 9,740
  • Joined: 08-August 08

Re: Is this good practice? (OOP/PDO)

Posted 20 April 2012 - 06:54 AM

True, but either way your code will fail if you pass it something other than a PDO object. If you see the constructor I'd expect that you're going to notice that it's a PDO object regardless of the hint. I guess the error returned will be helpful if you declare type.
Was This Post Helpful? 0
  • +
  • -

#7 Dormilich  Icon User is online

  • 痛覚残留
  • member icon

Reputation: 3394
  • View blog
  • Posts: 9,598
  • Joined: 08-June 10

Re: Is this good practice? (OOP/PDO)

Posted 20 April 2012 - 06:57 AM

View PostCTphpnwb, on 20 April 2012 - 03:54 PM, said:

I guess the error returned will be helpful if you declare type.

that’s the whole point (and you get the error on the first place where the error occurred). though the full potential of Type Hinting comes with Interfaces

This post has been edited by Dormilich: 20 April 2012 - 06:57 AM

Was This Post Helpful? 0
  • +
  • -

#8 cxn  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 33
  • Joined: 10-March 12

Re: Is this good practice? (OOP/PDO)

Posted 21 April 2012 - 04:53 AM

Let me ask another question; is OOP REALLY worth it? All it's doing now is piss me off :/
Was This Post Helpful? 0
  • +
  • -

#9 CTphpnwb  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 2834
  • View blog
  • Posts: 9,740
  • Joined: 08-August 08

Re: Is this good practice? (OOP/PDO)

Posted 21 April 2012 - 08:01 AM

Yes, without any doubt. OOP is about organization, and that should always be your primary goal when writing code. For most beginners the primary goal is to get their code to work and that's why they run into trouble. The more organized code is the easier it is to get it working, and conversely, the less organized the harder it is.
Was This Post Helpful? 0
  • +
  • -

#10 cxn  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 33
  • Joined: 10-March 12

Re: Is this good practice? (OOP/PDO)

Posted 21 April 2012 - 09:20 AM

View PostCTphpnwb, on 21 April 2012 - 08:01 AM, said:

Yes, without any doubt. OOP is about organization, and that should always be your primary goal when writing code. For most beginners the primary goal is to get their code to work and that's why they run into trouble. The more organized code is the easier it is to get it working, and conversely, the less organized the harder it is.


I understand that but the thing is, I've never really created any application yet, besides a register/login script with basic session work. I don't know if I have enough basic knowledge of php at this point :/ OOP is completely different from the things I've learned so far. It really messed up my brain, no kidding. I read here and there that OOP and PDO are two main things I should learn. PDO was ok, basic classes was ok (read a tutorial), but getting those two to work properly was impossible hah.. I don't kow what to do really
Was This Post Helpful? 0
  • +
  • -

#11 CTphpnwb  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 2834
  • View blog
  • Posts: 9,740
  • Joined: 08-August 08

Re: Is this good practice? (OOP/PDO)

Posted 21 April 2012 - 12:00 PM

Maybe a simple example will help. Here's code that generates an image with a random number of randomly sized, randomly positioned, and randomly colored rectangles. It's only 38 lines long, including spacer lines and PHP tags. It's also pretty easy to follow what's going on. I don't know how I'd do the same thing without using OOP. I might use functions by themselves, but I'm pretty sure it would be more complicated.
<?php
class box {
	protected $top;	
	protected $left;
	protected $width;
	protected $height;
	protected $box_color;
	protected $image;
	
	function __construct(&$im) {
		$this->image = $im;
		$this->top = rand(1,550);
		$this->left = rand(1,550);
		$this->width = rand(2,49);
		$this->height = rand(2,49);
		$this->box_color = imagecolorallocate($im, rand(0,240), rand(0,240), rand(0,240));

	}
	
	function draw_box() {
		imagefilledrectangle($this->image, $this->left, $this->top, $this->left + $this->width, $this->top + $this->height, $this->box_color);
	}

}

$im = imagecreate(600, 600) or die("Cannot Initialize new GD image stream");
$background = imagecolorallocate($im, 240, 240, 200);
imagefill($im,0,0,$background);
$boxes = array();
$n = rand(10,100);
for($i = 0; $i < $n; $i++) {
	$boxes[$i] = new box($im);
	$boxes[$i]->draw_box();
}

imagepng($im,"testing.png");
echo '<img src="testing.png" />';
?> 

Was This Post Helpful? 0
  • +
  • -

Page 1 of 1