How to refactor this code?

Question

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');
    }

Answer 1

Two things I found about Validate class:

1. It has methods for validations
2. It has block of code for querying the database.

Well, the code for querying the database can be put into different class. This is to make sure that concerns are separated (separation of concerns). This, in method, isPasswordAlsoIn, there will be code related with business rules and then the database check can be delegated to a separate class.

On a separate note, you may want to avoid SQL queries like following just to avoid SQL injection.

SELECT * FROM account WHERE email = '$value' AND password = '$value2'

Answer 2

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.

Answer 3

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.)