Comment refactoriser ce code?

Question

Groupé 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; 
     } 
    } 
?> 

Une partie de la classe Valider:

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

L'utilisation:

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

Essayez de trouver les similitudes et les différences entre les composants dans votre code. Par exemple, vous avez besoin d'un Form que vous avez déjà compris, mais le formulaire se compose de différents champs, alors pourquoi ne pas les extraire en un groupe de classes Field? Comme EmailField, PasswordField.

Vous avez probablement remarqué que Validate fait trop de choses. Par exemple, si un formulaire se compose uniquement d'un champ de courrier électronique, vous ne souhaitez pas que Validate comporte quoi que ce soit concernant les mots de passe ou autres. Lorsque vous commencez à ajouter des règles de validation pour les "noms d'utilisateur" ou "pays d'origine" ou tout autre attribut, vous ne souhaitez pas ajouter de règles à une grande classe Validate, mais à une classe d'assistance telle que ValidateValidateEmailField.

Answer 2

Ecrire (unité) teste pour vous assurer que votre code fonctionne correctement. Puis changez-le étape par étape et exécutez les tests après chaque étape. De cette façon, vous vous assurez que votre code fonctionne correctement après le refactoring.

La structure de test, par ex. PHPUnit

(je l'espère, vous ne vous attendiez le code refondus comme réponse.)

Answer 3

Deux choses que j'ai à propos de la classe: Valider

1. Il a des méthodes pour
2. validation Il a bloc de code pour interroger la base de données.

Eh bien, le code pour interroger la base de données peut être placé dans une classe différente. Ceci est de s'assurer que les préoccupations sont séparées (séparation des préoccupations). Ceci, dans la méthode, isPasswordAlsoIn, il y aura du code lié aux règles métier et ensuite la vérification de la base de données peut être déléguée à une classe séparée. Sur une note distincte, vous souhaiterez peut-être éviter les requêtes SQL comme suit pour éviter l'injection SQL. SELECT * FROM compte WHERE email = '$ valeur' ​​AND mot de passe = '$ valeur2'