[SOLVED] How to use OOP to check a form?

[piggeh]

Hi all

 

I am fairly new to PHP programming. I recently devised a registration script that checks the input of a form, checks against a database etc, then sends a verification link to an email address, before verifying the user. All worked fine. However I'm interesting in learning how to do this in an object based way rather than coding every single script. I got thus far (obviously this is fairly basic atm):

 

$user = $_POST["username"];
$pwd = $_POST["password"];
$pwd2 = $_POST["password1"];
$email = $_POST["email"];
$inputcheck = $_POST["inputcheck"];
$error;

include_once("formcheck.php");

$checker = new fieldcheck();
$checker->checkEmail($email);

 

The formcheck.php is as follows:

 

<?php

class fieldcheck {

var $error1;

function _constructor($n) {
//this needs to do checks that apply to all form field types - ie malicious data check
}

//checks for blank inputs
function valBlanks($n) {
if (empty($n)) {
$this->error1 .= "$n is blank <br />";
return $error1;
}
return;
}
function valEmail($n) {
if (!filter_var($n, FILTER_VALIDATE_EMAIL)) {
$this->error1 .= "email address is not valid <br />";
return $error1 ;
}
return;
}

function valMatch($n, $p) {
if ($n !== $p) {
$error1 .="passwords do not match <br />";
return £error1;
}
return;
}

function checkEmail($n) {
$this->valEmail($n);
$this->valBlanks($n);
}

function checkPasswords($n, $p) {
$this->valMatch($n, $p);
}
}

?>

 

I get an error on the above (something about 'expected syntax '`)" on line 13 of the formcheck.php script) but I could not see any obvious error. Also is my syntax correct, especially on the this->valMatch etc. Do I need to pass variables to objects in a certain way?

 

Any help appreciated, thank you.

[mikesta707]

well firstly, since your checkEmail function doesn't return anything, you won't ever see an error. If you want to check if the email passed or failed, you want to return true or false, and based on the output an error in the checkEmail function, (Or, as I like to do, create an error function for handling errors, and call that function with an error message)

 

Also, you cannot concatenate stuff onto the error1 string because it doesn't have any value stored into it. (You do this in your valEmail and valBlanks check. You may also want to do a trim on the input value just to make sure it isn't a bunch of spaces.

 

 

 

What line is line 13?

[piggeh]

Damn, forgot to do the verification and lost my post..

 

line 13 is

if (empty($n)) {

 

Would the variable (property?) $error1 not be accessible to the main script? Do I need to call an error handling function instead? If so how would this then return something to the main script?

 

Currently he main script acts like:

 

If hiddenvariablesubmittedwithform is empty, display form; If not then if error variable contains enything, display error and form again; if not, then process form.

[mikesta707]

having a string with an error message in it, and checking if that has data populated in it is not the best way to go about error handling IMO. I don't really even see where you check if that variable has data in it (unless you didn't post that part)

 

The class variable IS accessible to the rest of the class, and you don't NEED to call an error handling function, but your class would be a lot cleaner imo if it were to have a centralized error handling function. You could avoid writing duplicate code a bunch of time just to make sure there is no data in your objects error variable, and you could avoid having to clear that variable of data everytime you do something new.

 

line 13 looks perfectly fine to me tho... What exactly is the error message you get.

[piggeh]

My stuff is on my memory stick at the mo, will get it up and running at work tomorrow and copy the error message

[piggeh]

having a string with an error message in it, and checking if that has data populated in it is not the best way to go about error handling IMO. I don't really even see where you check if that variable has data in it (unless you didn't post that part)

 

The class variable IS accessible to the rest of the class, and you don't NEED to call an error handling function, but your class would be a lot cleaner imo if it were to have a centralized error handling function. You could avoid writing duplicate code a bunch of time just to make sure there is no data in your objects error variable, and you could avoid having to clear that variable of data everytime you do something new.

 

line 13 looks perfectly fine to me tho... What exactly is the error message you get.

 

I load it up today and error doesn't occur. I think xampp was not refreshing the script or sometihng strange was happening.

 

I'm not too sure how an error handling function would work but I will try and look up an example. I guess it's easy enough if I just want the error to be printed but ideally want to display errors and reload the form at the same time.

[piggeh]

OK - I have made a couple of changes..

 

registrationoop.php

<?php
$user = $_POST["username"];
$pwd = $_POST["password"];
$pwd2 = $_POST["password1"];
$email = $_POST["email"];
$inputcheck = $_POST["inputcheck"];
$error;

include_once("formcheck.php");

//check for previous input. if exists, check input using formcheck.php methods
if ($inputcheck==1) {
echo "checking inputs..";
$checker = new fieldcheck();
//check for erros from above, if all reutrned true then process form
if ($checker->checkEmail($email)) {
echo "processing form";
//process form
}
else {
echo $checker->getError;
echo "error encountered..";
//show form again if error variable contains values ie error messages
//consider moving form and using include_once() to allow seperation of elements
?>
<form action="registrationoop.php" method="post" >
username:<input type="text" name="username" value="<?php echo($_POST["username"]); ?>" /><br />
password:<input type="password" name="password" /><br />
re-enter password:<input type="password" name="password1" /><br />
email:<input type="text" name="email" value="<?php echo($_POST["email"]); ?>" /><br />
<input type="hidden" name="inputcheck" value="1" /><br />
<p><input type="submit" value="submit" />
<?php
}
//check for input with hidden input variable
}
else {
//else, no input or previous visit, display form
?>
<form action="registrationoop.php" method="post" >
username:<input type="text" name="username" /><br />
password:<input type="password" name="password" /><br />
re-enter password:<input type="password" name="password1" /><br />
email:<input type="text" name="email" /><br />
<input type="hidden" name="inputcheck" value="1" /><br />
<p><input type="submit" value="submit" />
<?php
}
?>

 

formcheck.php

 

<?php

class fieldcheck {

var $testdata;
var $inputerror = "test";

function _constructor($n) {
//this needs to do checks that apply to all form field types - ie malicious data check
}

//checks for blank inputs
function valBlanks($n) {
if (empty($n)) {
$this->setError("required field is blank <br />");
return false;
}
else {
return true;
}
}

function valEmail($n) {
if (!filter_var($n, FILTER_VALIDATE_EMAIL)) {
$this->setError("email address is not valid <br />");
return false;
}
else {
return true;
}
}

function valMatch($n, $p) {
if ($n !== $p) {
$this->setError("passwords do not match<br />");
return false;
}
else {
return true;
}
}

function checkEmail($n) {
if($this->valEmail($n) && $this->valBlanks($n)) {
return true;
}
else {
return false;
}
}

function checkPasswords($n, $p) {
$this->valMatch($n, $p);
}
function setError($n) {
$this->inputerror = $n;
}

function getError() {
return $this->inputerror;
}
}

?>

 

I have firstly changed it so rather than checking for error variables it checks if functions evaluate to true or false. At the moment it just checks email address and no others as I thought it's best to get one field working before implementing the rest.

 

It seems to evaluate correctly - if I fill in an email address it will print 'processing form'. However when I leave it blank it prints 'an error has occurred' but does not output the errors. Now, I know setErrors will mean only one error is stored but it's not printing that error and not even printing 'test' (which I put as a default value for the variable to check if it outputs).

 

Where am I going wrong?

 

[KevinM1]

Your basic OOP code is off.  If you're using PHP 5 (which, really, you should if you want to use PHP) then there are a couple of things that immediately jump out.

 

1. The use of 'var' when declaring data members.  You should use one of the access keywords - public, protected, or private - instead.  In most cases, you'll want to use private to force client code to use your accessor methods (like get/setError()) in order to manipulate object data.

 

2. Your constructor is wrong.  It's supposed to be:

 

public function __construct(/* argument list */)
{
   // constructor code
}

 

So, that's two underscores, and the function name 'construct', not 'constructor'.

 

Start with those fixes, and try again.

[piggeh]

Thanks - I have updated (and taken out testdata variable as wasnt being used) - the construct class is a bit redundant as well really at the moment - but it still does the same as before (no error messages)

[KevinM1]

Thanks - I have updated (and taken out testdata variable as wasnt being used) - the construct class is a bit redundant as well really at the moment - but it still does the same as before (no error messages)

 

Show your new code.

[piggeh]

code is now:

 

<?php

class fieldcheck {

private $inputerror = "test";

function __construct() {
//this needs to do checks that apply to all form field types - ie malicious data check
}

//checks for blank inputs
function valBlanks($n) {
if (empty($n)) {
$this->setError("required field is blank <br />");
return false;
}
else {
return true;
}
}

function valEmail($n) {
if (!filter_var($n, FILTER_VALIDATE_EMAIL)) {
$this->setError("email address is not valid <br />");
return false;
}
else {
return true;
}
}

function valMatch($n, $p) {
if ($n !== $p) {
$this->setError("passwords do not match<br />");
return false;
}
else {
return true;
}
}

function checkEmail($n) {
if($this->valEmail($n) && $this->valBlanks($n)) {
return true;
}
else {
return false;
}
}

function checkPasswords($n, $p) {
$this->valMatch($n, $p);
}
function setError($n) {
$this->inputerror = $n;
}

function getError() {
return $this->inputerror;
}
}

?>

 

I guess if I passed enough info to it on instantiation I could skip the $check->checkEmail() etc lines on the registration form but will save that for another day, just want to get this working properly first. I looked up my notes and the set/get error part is almost lifted the same as from the book (learning from SAM's teach yourself php in 24 hours initially).

[KevinM1]

Okay, let's start slow with the code.  Try the following code, just to get started:

 

class InputValidator
{
   private $errors = array();

   public function __construct(){}

   public function getErrors(){ return $this->errors; }
   public function setError($error){ $this->errors[] = $error; }
   public function printErrors()
   { 
      foreach($this->errors as $error)
      { 
         echo $error . "<br />";
      }
   }
}

$validator = new InputValidator();
$validator->setError("test");
$validator->printErrors();

 

Let me know if the word 'test' is output.

[piggeh]

Okay, let's start slow with the code.  Try the following code, just to get started:

 

class InputValidator
{
   private $errors = array();

   public function __construct(){}

   public function getErrors(){ return $this->errors; }
   public function setError($error){ $this->errors[] = $error; }
   public function printErrors()
   { 
      foreach($this->errors as $error)
      { 
         echo $error . "<br />";
      }
   }
}

$validator = new InputValidator();
$validator->setError("test");
$validator->printErrors();

 

Let me know if the word 'test' is output.

 

Thanks.

 

I updated my code and it works fine now, many thanks for the guidance. Seems I spent too long between studiyng and doing sometihng practical and have forgotten some of the basics!