PHP array returns nothing (OOP newbie needs some help)

I am 开发者_开发知识库trying to create an form validator class that will check to make sure that people have typed a particular number of characters into specified fields. Here is my page:

if ($_SERVER['REQUEST_METHOD'] == 'POST') { 
    require_once('functions.php');
    require_once('formValidator.php');


    $validator = new FormValidator();
    $validator->setFirst($_POST['first']);
    $validator->setLast($_POST['last']);
    $validator->setEmail($_POST['email']);

    $errors = array();
    $validator->checkLength($errors);
    echo $errors;


    if (!empty($errors)) {
        echo '<p>' . $message . '</p>';
    }
}

// form here (code above is in PHP tags, of course)

Here is my class:

class FormValidator {

    public $first;
    public $last;
    public $email;
    public $fields_with_lengths;

    public function setFirst($first) {   
        $this->first = $first; 
    }

    public function setLast($last) {   
        $this->last = $last; 
    }

    public function setEmail($email) {   
        $this->email = $email; 
    }

    function setLengths() {
        $this->fields_with_lengths = array('first' => 2, 'last' => 2, 'email' => 8);
    }

    function checkLength($fields_with_lengths) {
        $length_errors = array();

        foreach($fields_with_lengths as $fieldname => $maxlength) {
            if (strlen(trim($_POST[$fieldname])) > $maxlength) {
                $length_errors[] = $fieldname;
            }
        }

        if (!empty($length_errors)) {
            $message = 'There were no errors';
        } else {
            $message = 'There were errors';

        }
        return $message;
    }

}

$validator = new FormValidator();
$validator->setFirst($_POST['first']);
$validator->setLast($_POST['last']);
$validator->setEmail($_POST['email']);

Can anyone see what I am doing wrong? I'd be grateful for some help. Thanks.

You're passing an empty array into checkLength(), then iterating over it (so the for loop will never have anything to iterate over). You're returning $message from checkLength() but not assigning it to anything.

$errors = array('first' => 8,
                'last' => 8,
                'email' => 16
               );
$message = $validator->checkLength($errors);

In PHP by default all values to functions are passed as values, not references (unless they are objects).

First of all, if you want to use a variable as a return value, you'd have to define checkLength($var) as:

function checkLength(&$fields_with_lengths) { /* ... */ }

You could in PHP4 do a call like checkLength(&$errors), but that's deprecated as of PHP5 and will throw an E_STRICT warning. Of course, you still can (but that functionality may be thrown away soon), though the "correct" way is, as said, giving it as reference.

An even more elegant way of acting, and what should really be done in PHP, is to return the $fields_with_lengths. This way:

function checkLength() {
  $var = array();
  // Your code
  return $var;
}

And in the call to the function, just say

$return = $validator->checkLength();

But that does not solve the logic of your problem.

You are iterating over a parameter which is an empty array. You are not iterating over $validator->fields_with_lengths. Even more, $fields_with_lengths, in your code is not initialized ever.

You would have to call $validator->setLengths(). A better way of doing is (you are using PHP5) declaring a constructor, which in PHP5 is done with the magic function __construct().

However, as you are just initializing the variable always with the same constant values, you can do that in the very definition, just like this:

So you would have something like:

class FormValidator {

    pubilc $first;
    pubilc $last;
    pubilc $email;
    pubilc $fields_with_lengths = array('first' => 2, 'last' => 2, 'email' => 8);

    // Rest of code
}

What's more.. ¿Why are the attributes marked as public?

OOP theory tells us to close the scope of visibility as much as possible. And as you have setters, you don't need to access the attributes directly. You can in PHP have a magic__get($var)to return a private $attribute if you want the syntactic sugar ofecho $validator->first;`.

Example:

function __get($var) {
    if(isset($this->{$var})) return $this->{$var}; // this $this->{$var} could even be written as $this->$var but I did it like that for clarity.
}

If you also want the syntactic sugar of $validator->first = "John"; you can do the same with the setter, as in:

function __set($var, $value) {
    if(isset($this->{$var})) {
        return $this->{$var} = $value;
    } return null; // Or you can throw an exception if you like
}

In PHP, as opposed to other languages, such as Java, you must explicitly use $this as the current instance of an object within a function.

So your checkLength is misdefined.

You should also do something with the return value, instead of just dropping it.

With just that minor modifications, you could fix your code with

function checkLength() {
    $length_errors = array();

    foreach($this->fields_with_lengths as $fieldname => $maxlength) {
        if (strlen(trim($_POST[$fieldname])) > $maxlength) {
            $length_errors[] = $fieldname;
        }
    }

    if (!empty($length_errors)) {
        $message = 'There were no errors';
    } else {
        $message = 'There were errors';

    }
    return array($length_errors, $message);
}

You were using a variable ($length_errors otherwise inaccessible! so you were computing data which would never be useful).

And in the call to the function, something like $validator = new FormValidator();

// You could leave the setXXX($_POST[xxx])

$validator->first = $_POST['first'];
$validator->last = $_POST['last'];
$validator->email = ($_POST['email'];

$errors = $validator->checkLength();
echo "<p> {$errors[1]} </p>";

if(!empty($errors[0])) {
    // You could do something like foreach($errors[0] as $k => $fieldname) { echo "Error in $fieldname<br />" }
}

$errors is empty because you initialize it but never change it. $message is unset because you never assign anything to it.

You're defining $errors = array();, making the errors an empty array. Nowhere do you attempt to set it to any other value, thus it remains an empty array.