views:

121

answers:

2

Grouped params:

<?php
class Form {
    private $field;

    public function getFieldRelated($field) {
        return $this->fieldrelated[$field];
    }

    public function __construct() {
        $this->fieldrelated['email']['name'] = 'email';
        $this->fieldrelated['email']['value'] = $_POST['email'];
        $this->fieldrelated['email']['pattern'] = REGEX_EMAIL;
        $this->fieldrelated['email']['confirmation'] = 'emailconfirmation';
        $this->fieldrelated['email']['names'] = 'emails';

        $this->fieldrelated['emailconfirmation']['name'] = 'email confirmation';
        $this->fieldrelated['emailconfirmation']['value'] = $_POST['emailconfirmation'];
        $this->fieldrelated['emailconfirmation']['pattern'] = REGEX_EMAIL;

        $this->fieldrelated['password']['name'] = 'password';
        $this->fieldrelated['password']['value'] = $_POST['password'];
        $this->fieldrelated['password']['pattern'] = REGEX_PASSWORD;
        $this->fieldrelated['password']['confirmation'] = 'passwordconfirmation';
        $this->fieldrelated['password']['names'] = 'passwords';

        $this->fieldrelated['passwordconfirmation']['name'] = 'password confirmation';
        $this->fieldrelated['passwordconfirmation']['value'] = $_POST['passwordconfirmation'];
        $this->fieldrelated['passwordconfirmation']['pattern'] = REGEX_PASSWORD;
        }
    }
?>

A part of the Validate class:

public function isEmpty($field) {
    $fieldrelated = $this->form->getFieldRelated($field);
    $name = $fieldrelated['name'];
    $value = $fieldrelated['value'];

    if(empty($value)) {
        $this->setProperty($field, 'empty');
        $this->addErrorMessage('The '.$name.' is empty!');
        return true;
    } else {
        $this->setProperty($field, 'unempty');
        return false;
    }
}
public function isValid($field) {
    $fieldrelated = $this->form->getFieldRelated($field);
    $name = $fieldrelated['name'];
    $value = $fieldrelated['value'];

    $pattern = $fieldrelated['pattern'];

    if(preg_match($pattern, $value)) {
        $this->setProperty($field, 'valid');
        return true;
    } else {
        $this->setProperty($field, 'invalid');
        $this->addErrorMessage('The '.$name.' is invalid!');
        return false;
    }
}
public function isConfirmed($field) {
    $fieldrelated = $this->form->getFieldRelated($field);
    $value = $fieldrelated['value'];

    $field2 = $fieldrelated['confirmation'];

    $fieldrelated2 = $this->form->getFieldRelated($field2);
    $value2 = $fieldrelated2['value'];

    $names = $fieldrelated['names'];

    if($value == $value2) {
        $this->setProperty($field, 'confirmed');
        $this->setProperty($field2, 'confirmed');
        return true;
    } else {
        $this->setProperty($field, 'unconfirmed');
        $this->setProperty($field2, 'unconfirmed');
        $this->addErrorMessage('The '.$names.' are unconfirmed!');
        return false;
    }
}
public function isEmailOnlyIn($correct) {
    $fieldrelated = $this->form->getFieldRelated('email');
    $name = $fieldrelated['name'];
    $value = $fieldrelated['value'];

    $value = mysql_real_escape_string($value);
    $result = "SELECT * FROM account WHERE email = '$value'";
    $result = mysql_query($result);
    $result = mysql_fetch_array($result);

    if($result) {
        $this->setProperty('email', 'email only in');
        if($correct == 'not in') {
            $this->addErrorMessage('The '.$name.' is in database!');
        }
        return true;
    } else {
        $this->setProperty('email', 'email only not in');
        if($correct == 'in') {
            $this->addErrorMessage('The '.$name.' is not in database.');
        }
        return false;
    }
}
public function isPasswordAlsoIn($correct) {
    $fieldrelated = $this->form->getFieldRelated('email');
    $name = $fieldrelated['name'];
    $value = $fieldrelated['value'];

    $fieldrelated2 = $this->form->getFieldRelated('password');
    $name2 = $fieldrelated2['name'];
    $value2 = $fieldrelated2['value'];

    $value = mysql_real_escape_string($value);

    $value2 = md5($value2);
    $value2 = mysql_real_escape_string($value2);

    $result = "SELECT * FROM account WHERE email = '$value' AND password = '$value2'";
    $result = mysql_query($result);
    $result = mysql_fetch_array($result);

    if($result) {
        $this->setProperty('password', 'password also in');
        if($correct == 'not in') {
            $this->addErrorMessage('The '.$name2.' is in database!');
        }
        return true;
    } else {
        $this->setProperty('password', 'password also not in');
        if($correct == 'in') {
            $this->addErrorMessage('The '.$name2.' is not in database!');
        }
        return false;
    }
}

The usage:

    if(!$validate->isEmpty('email')) {
        $validate->isValid('email');
    }
    if(!$validate->isEmpty('emailconfirmation')) {
        $validate->isValid('emailconfirmation');
    }
    if($validate->isProperty('email', 'valid') && $validate->isProperty('emailconfirmation', 'valid')) {
        $validate->isConfirmed('email');
    }

    if(!$validate->isEmpty('password')) {
        $validate->isValid('password');
    }
    if(!$validate->isEmpty('passwordconfirmation')) {
        $validate->isValid('passwordconfirmation');
    }
    if($validate->isProperty('password', 'valid') && $validate->isProperty('passwordconfirmation', 'valid')) {
        $validate->isConfirmed('password');
    }

    if($validate->isProperty('email', 'confirmed') && $validate->isProperty('emailconfirmation', 'confirmed')) {
        $validate->isEmailOnlyIn('not in');
    }
+2  A: 

Write (unit) tests to ensure that your code is working correctly. Than change it step by step and run the tests after each step. This way you ensure that your code is working properly after the refactoring.

Test framework, e.g. PHPUnit

(I hope you didn't expect the refactored code as an answer.)

Felix Kling
+1  A: 

Try to find the similarities and differences between components in your code. For example, you need a Form which you have already figured out, but the form consists of different fields, so why not extract them into a bunch of Field-classes? Like EmailField, PasswordField.

You have probably noticed that Validate does too many things. For example, if a form only consists of an email field, you don't want Validate to include anything regarding passwords or the like. When you start adding validation rules for "usernames" or "country of origin" or any other attribute, you don't want to be adding rules to a big, single Validate-class, but either to each Field or a helper class such as ValidateEmailField.

chelmertz