Use form to create user object

  • (2 Pages)
  • +
  • 1
  • 2

23 Replies - 8841 Views - Last Post: 28 July 2011 - 08:17 AM Rate Topic: -----

#1 DeWire  Icon User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 73
  • Joined: 06-December 08

Use form to create user object

Posted 26 July 2011 - 02:43 AM

Hi guys,

I have a class called user that creates a user with a username password and email (3 variables) and adds the details to a database. I have tested this class using $user = new user("username", "password", "email"); and it works perfect.

OK, now I want to initiate this class from a form. I have my form set so that when the form submit button is pressed :
$user = new user("$_POST['username'], $_POST['password'], $_POST['email']; 


but, as you suspected, this isnt working for me. Could anyone shed some light on this for me ?

Here is my class file (Im new to using classes and want to try and get a better understanding of them, so if you see anything wrong, I would appreciate an explanation as to why it is causing problems )

<?php


class user  {

	var $username;
	var $password;
	var $email;

	function __construct($username, $password, $email) {

		$this->setValue("username", $username);
		$this->setValue("password", $password);
		$this->setValue("email", $email);
		$this->addUser();

	}

	function setValue($type, $value)  {
		$this->$type = $value;
	}

	function addUser() {
		$conn = mysql_connect("localhost", "root", "") or DIE("Failed to Connect!");
        $db = mysql_select_db("test");

        $query = "INSERT INTO test VALUES('$this->username', '$this->password', '$this->email')";
        $result = mysql_query($query) OR DIE("Query Failed : " . mysql_error());
        $close = mysql_close($conn);
	}



}






All help appreciated guys,

Thanks,
Dave

This post has been edited by DeWire: 26 July 2011 - 02:44 AM


Is This A Good Question/Topic? 0
  • +

Replies To: Use form to create user object

#2 Dormilich  Icon User is offline

  • 痛覚残留
  • member icon

Reputation: 3550
  • View blog
  • Posts: 10,319
  • Joined: 08-June 10

Re: Use form to create user object

Posted 26 July 2011 - 03:00 AM

is the $_POST array populated correctly?

further there are some potential problems in your code:
- line #6 – #8, the var keyword stems from PHP 4 and it should not be used in PHP 5. PHP 5 uses one of the visibility keywords (public, protected, private). those keywords also apply to the method defintions (defaults to public)
- line #24, die() is not sign of good error handling, as it will mess up your HTML output
- line #24 – #29, the mysql_* functions are outdated, use PDO or MySQLi instead
- line #27, an INSERT query should define the order of the columns, to which the values go. the query might invalidate if there is a change to the table structure.
- line #27, complex values like array elements or object properties should be wrapped in curly braces to denote the full variable. it would be better to use sprintf() in these cases.

from the design perspective, each denoted property should have its own setter/getter method.

This post has been edited by Dormilich: 26 July 2011 - 03:03 AM

Was This Post Helpful? 2
  • +
  • -

#3 DeWire  Icon User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 73
  • Joined: 06-December 08

Re: Use form to create user object

Posted 26 July 2011 - 03:24 AM

Hi Dormilich,
Thanks for your input.

However, this is only something simple im working on - trying to get my head around it. Hence there are no getters/setters and there are die functions.

In terms of the form not functioning the way I want it to , are you suggesting that the problem lies with my $_POST array?

Thanks,
Dave
Was This Post Helpful? 0
  • +
  • -

#4 codeprada  Icon User is offline

  • Changed Man With Different Priorities
  • member icon

Reputation: 947
  • View blog
  • Posts: 2,355
  • Joined: 15-February 11

Re: Use form to create user object

Posted 26 July 2011 - 04:02 AM

When you're initializing the class here
$user = new user("$_POST['username'], $_POST['password'], $_POST['email']; 
you've got an extra quote and no closing parenthesis.

Like Dormilich said getter and setters, they make your life easier. You've got a somewhat Java approach.

This post has been edited by codeprada: 26 July 2011 - 04:02 AM

Was This Post Helpful? 2
  • +
  • -

#5 DeWire  Icon User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 73
  • Joined: 06-December 08

Re: Use form to create user object

Posted 26 July 2011 - 04:10 AM

Thanks guys. Will let you know how I get on :)

Can't believe I missed that...

Dave

Oops, turns out that was a typo when I was writing the code into Dream.In.Code.

My own version does infact have the correct syntax for the new user :

$user = new user($_POST['username'], $_POST['password'], $_POST['email']);


Will have another look at it myself, see if I can figure something out,

Regards,
Dave
Was This Post Helpful? 0
  • +
  • -

#6 macosxnerd101  Icon User is offline

  • Self-Trained Economist
  • member icon




Reputation: 10647
  • View blog
  • Posts: 39,539
  • Joined: 27-December 08

Re: Use form to create user object

Posted 26 July 2011 - 04:12 AM

View PostDormilich, on 26 July 2011 - 06:00 AM, said:

- line #6 – #8, the var keyword stems from PHP 4 and it should not be used in PHP 5. PHP 5 uses one of the visibility keywords (public, protected, private). those keywords also apply to the method defintions (defaults to public)

And unless you have good reasons for using otherwise, stick to the private modifier for your fields. It goes towards encapsulation.
Was This Post Helpful? 0
  • +
  • -

#7 DeWire  Icon User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 73
  • Joined: 06-December 08

Re: Use form to create user object

Posted 26 July 2011 - 04:32 AM

OK guys,got my form working with the class file. I would just like to explain what I have done to see whether or not I have done it proper.

I have my form now posting to user.php rather than $self.

Beneath the class within user.php I have the following :

 $user = new user($_POST['username'], $_POST['password'], $_POST['email']); 


Is this programmatically correct?

Thanks for all help thus far, I will take peoples thoughts into consideration for official projects,

Regards,
Dave
Was This Post Helpful? 0
  • +
  • -

#8 Dormilich  Icon User is offline

  • 痛覚残留
  • member icon

Reputation: 3550
  • View blog
  • Posts: 10,319
  • Joined: 08-June 10

Re: Use form to create user object

Posted 26 July 2011 - 04:33 AM

View PostDeWire, on 26 July 2011 - 12:24 PM, said:

In terms of the form not functioning the way I want it to , are you suggesting that the problem lies with my $_POST array?

since I don’t know what is in your $_POST array (never make assumption while debugging!), checking its content is the first thing to do.
Was This Post Helpful? 0
  • +
  • -

#9 JackOfAllTrades  Icon User is offline

  • Saucy!
  • member icon

Reputation: 6075
  • View blog
  • Posts: 23,540
  • Joined: 23-August 08

Re: Use form to create user object

Posted 26 July 2011 - 04:49 AM

You're also using raw user input, which opens you up to a world of hurt via SQL Injection.
Was This Post Helpful? 1
  • +
  • -

#10 Dormilich  Icon User is offline

  • 痛覚残留
  • member icon

Reputation: 3550
  • View blog
  • Posts: 10,319
  • Joined: 08-June 10

Re: Use form to create user object

Posted 26 July 2011 - 05:21 AM

we recommend to use Prepared Statements via PDO to counter these (Intro tutorial, and more in the PHP Tutorial Section)
Was This Post Helpful? 1
  • +
  • -

#11 satis  Icon User is offline

  • D.I.C Head

Reputation: 82
  • View blog
  • Posts: 231
  • Joined: 26-May 11

Re: Use form to create user object

Posted 26 July 2011 - 05:36 AM

Some additional feedback

1. You're not checking to see if the user exists before trying to insert it. Having multiple users with the same username is probably a bad thing.
2. You're storing your password in plain-text. You should never store a password in plain-text... always one-way hash it. http://www.php.net/m...nction.hash.php You'll probably want to use a salt as well, but that's probably getting a bit further than you're interested in.

I'd also like to reinforce the use of PDO or MySQLi. You may as well learn the right way immediately... it's no harder to learn than the old mysql_* functions, but way better.
Was This Post Helpful? 1
  • +
  • -

#12 DeWire  Icon User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 73
  • Joined: 06-December 08

Re: Use form to create user object

Posted 26 July 2011 - 06:40 AM

OK guys,

I've taken what you have all said into consideration, this is now my class file:



<?php


class user  {

	private $username;
	private $password;
	private $email;

	function __construct($username, $password, $email) {
		$this->setValue("username", sha1($username));
		$this->setValue("password", $password);
		$this->setValue("email", $email);
		$this->addUser($this->username, $this->password, $this->email);
	}

	function setValue($type, $value)  {
		$this->$type = $value;
	}

	function addUser($user, $pass, $email) {
        $conn = new PDO("mysql:host=localhost;dbname=test", "root", "");
        $conn->exec("INSERT INTO test (col1, col2, col3) values ('$user', '$pass', '$email' )");
	}

}


    $user = new user($_POST['username'], $_POST['password'], $_POST['email']);




I know I havent used prepared statements, I am currently in the process of studying them.

This is my form reg.php


<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<title>Register!</title>
</head>
<body>

<form method="POST" action="user.php">
<label for="username">Username : </label>
<input type="text" name="username" id="username"/>

<br />

<label for="password">Password : </label>
<input type="password" name="password" id="password"/>

<br />

<label for="email">Email : </label>
<input type="text" name="email" id="email"/>

<br />

<input type="submit" name="submit" id="submit"/>

</form>
</body>
</html>




Is this programmatically correct?

Cheers for all help this far.

Regards,
Dave

This post has been edited by DeWire: 26 July 2011 - 06:44 AM

Was This Post Helpful? 0
  • +
  • -

#13 macosxnerd101  Icon User is offline

  • Self-Trained Economist
  • member icon




Reputation: 10647
  • View blog
  • Posts: 39,539
  • Joined: 27-December 08

Re: Use form to create user object

Posted 26 July 2011 - 06:50 AM

You don't pass your variables directly to the exec() function. Instead, you use placeholders to signify values. As an example:
$statement = $pdo->prepare("SELECT * FROM table_name WHERE username = ?");
$statement->execute(array("macosxnerd101"));


Was This Post Helpful? 1
  • +
  • -

#14 codeprada  Icon User is offline

  • Changed Man With Different Priorities
  • member icon

Reputation: 947
  • View blog
  • Posts: 2,355
  • Joined: 15-February 11

Re: Use form to create user object

Posted 26 July 2011 - 06:53 AM

Check out Dormilich's tutorial on PDO.
Was This Post Helpful? 0
  • +
  • -

#15 satis  Icon User is offline

  • D.I.C Head

Reputation: 82
  • View blog
  • Posts: 231
  • Joined: 26-May 11

Re: Use form to create user object

Posted 26 July 2011 - 07:37 AM

What you've got is definitely an improvement. Great work... keep at it.

Your universal setter works, but I personally prefer a separate getter/setter for each property. That allows you to easily add specific processing. For instance, when setting the password, the password could be automatically hashed within the setter. Here's the password getter/setter for a user class I'm working with.

	public function password($value = -1){
		if($value == -1)
			return $this->password;
		else
			$this->password = tools::hash($value, $this->salt);
	}


I don't know if people would frown upon my use of a single method as both getter and setter. *shrug* Still, you can see how this could be handy. If you were to include emails, the setter part could have your logic to validate the email, for instance.

btw, in your latest example, you sha1 the username instead of the password. :P I'd also recommend against either SHA1 or MD5. Both have problems with collisions. Here's the hash function I'm currently using.

	public static function hash($text, $salt = ''){
		return hash('sha512', $text .$salt);
	}


SHA512 doesn't have the collision problems that md5 and sha1 have (as far as I know, at least).
Was This Post Helpful? 0
  • +
  • -

  • (2 Pages)
  • +
  • 1
  • 2