Refactoring procedural vers poo

Par neecride, il y a 3 ans

PHP

Bonjour,

je commence doucement mon réfactoring j'ai commencer par le plus gros morceau, la pagination mais c'est pas pour ça que je poste car elle fonctionne super bien :)

Ici je commence ma class Form avec une class Validator.

voilà ce que j'ai fais dans un premier temps, ici je suis sur la page viewtopic de mon forum

j'y appel ma class form

$form = new App\Form(); $form->postReponse();

et ma class Validator je l'instancie dans ma class Form

class Form { private function validation() { return new Validator(); } //under construct public function postReponse($post){ if(isset($_POST[$post])){ $match = $this->GetRoute()->matchRoute(); //$this->validation()->checkCsrf(); $id = (int) $match['params']['id']; $content = strip_tags(trim($_POST['f_topic_content'])); //$userid = (int) $_SESSION['auth']->id; $this->validation()->Content($content,100,'pas bon'); $this->validation()->Content('test',100,'test error'); } } }

Ma class Validator j'ai fait trois function

private function errors(array $errors = []) { if(!empty($errors)){ foreach($errors as $v): return "<li class='errmode'>$v</li>"; endforeach; } } public function checkError() { return !empty($errors) ? "<div class='notify notify-rouge'><div class='notify-box-content'><ul>".$errors."</ul></div></div>" : '' ; } public function Content($field,$limit,$error) { if(grapheme_strlen($field) < $limit){ return $this->errors([$error]); } }

Ma fonction content envoie les erreur dans la fonction errors jusqu'ici ça va si je fais un var_dump j'ai bien mes erreurs, le truc est que je voudrais récupéré les erreurs de la fonction errors dans la fonction checkError qui sera echo dans mon template car j'y ai mis une class css qui fais que les erreur seront en position fixed avec un peut de js un peut comme sur grafikart, alors avec le foreach forcement ça bug ça se place les une sur les autres d'où la fonction checkError.

Mes erreur fonctionné avant le réfactoring parce que la variable erreur était accessible checkError(isset($erreur) ? $erreur : '' ); mais voilà on est dans le poo le but est de viré les script des vues.

j'ai essayer pleins de chose la fonction $validator->checkError(); ne dois pas avoir de parametres, j'ai essayer le constructeur mais je dois y passer des paramettres a chaque fois que je l'instancie.

Donc voilà la pagination c'était facile lol

bonne soirée.

8 réponses

JoolsMcFly, il y a 3 ans

Salut.

Il y a plusieurs choses à revoir.

Quand tu fais

$this->validation()->Content($content,100,'pas bon'); $this->validation()->Content('test',100,'test error');

ça appelle validation() qui renvoit à chaque fois une nouvelle instante de Validator. Je pense que tu veux UNE instance pour toute la validation, non ? Dans ce cas tu mets l'instantiation dans le contructeur :

private Validator $validator; public function __construct() { $this->validator = new Validator(); }

et au lieu d'appeler $this->validation()->Content tu appelles $this->validator->Content.

Ensuite :

public function postReponse($post){ if(isset($_POST[$post])){

Il serait mieux de passer les valeurs à vérifier en paramètres plutôt que d'utiliser directement $_POST dans la méthode.
Idem avec $content = strip_tags(trim($_POST['f_topic_content']));. Passe le contenu en param car si jamais tu décides de renommer f_topic_content il va falloir changer $_POST['f_topic_content'] partout.

Ensuite

public function checkError() { return !empty($errors) ? "<div class='notify notify-rouge'><div class='notify-box-content'><ul>".$errors."</ul></div></div>" : '' ; }

ici $errors n'est pas défini, car la variable $errors n'est pas déclarée dans la portée de la fonction, donc ça sera toujours considéré comme empty et la fonction retournera toujours ''.

Attention avec les return dans les foreach : ça va sortir de la fonction au premier passage :

private function errors(array $errors = []) { if(!empty($errors)){ foreach($errors as $v): return "<li class='errmode'>$v</li>"; // <======= si t'as 5 éléments dans $errors alors ça n'en traitera qu'un car tu as un return endforeach; } }

Enfin, ton validator ne devrait pas faire de formattage, c'est à dire ne pas générer de HTML car tu peux imaginer utiliser ce validator dans un appel API qui va renvoyer du JSON.
Je changerais donc la méthode errors comme suit :

private function errors(array $errors = []) { array_push($this->errors, ...$errors); // ajoute chaque erreur à la fin du tableau $this->errors }

Je pense qu'il faudrait que tu te couches tout ça sur papier et que tu repenses à ce que tu veux faire. J'ai l'impression que c'est encore un peu flou pour toi.
Une technique que je donne souvent c'est d'essayer d'expliquer ce que tu veux faire à un enfant de 8 ans. Ça permet de vulgariser et d'aller à l'essentiel.

En gros le scénario de ton appli est le suivant :

  1. utilisateur envoie un formulaire
  2. un Validator vérifie les données
  3. si pas d'erreur ($validator->isRequestValid() ou $validator->hasErrors() par exemple) alors tu peux enregistrer les données en base.
  4. sinon on affiche les erreurs : récup des messages erreurs puis la vue les affiche en rouge et en gras par exemple.

À un enfant de 8 ans : je demande des informations à un utilisateur et si c'est bon je les garde, sinon je lui dis ce qui ne va pas.

Voilà, j'espère que ça t'aide un peu. :)

neecride, il y a 3 ans

Bonsoir,

Ok je vois, je pensé pas qu'on pouvais instancier une fonction dans un constructeur en vrai je vois pas se que ça change, la logique n'est pas si simple en poo je pourrais prendre un truc tout fais mais je préfère apprendre a le faire, je vais suivre ta formule après c'est pas censé géré tout les cas ni être déporté dans un autre projet.

merci.

JoolsMcFly, il y a 3 ans

Note que tu n'instancies pas une fonction mais une classe.

neecride, il y a 3 ans

Bonjour,

je reviens parce que j'ai réfléchie a ce que je voulais, ici je séparé les choses avec une class Validator et une class Form déjà là pour moi c'est pas logique, j'ai donc recentralisé tout dans une seule class Validator qui validera les inputs et elle n'a pas besoin paramêtre dans le constructeur puisse que toutes les variables seront initialisé a l'interieur de la class.

Donc ce que je souhaite c'est la chose suivant,

j'initialise le match erreur sur mon template ou peut import a l'index puisse que tout est redirigé ici et <?= $validator->checkError(); ?> il est important qu'elle ne prenne pas de paramêtre elle va juste rendre tout le code de l'erreur interne traité par la class Validator avec une div qui se place en position fixed, c'est son seul job.

Et dans mes formulaire je n'appel que la fonction qui valide un element comme le contenu d'un topic ou d'une réponse etc... tout sera validé ou non par la class Validator.

Donc si j'ai un champ titre je ferai une fonction titre ou input peut import, je ferais plusieur fonction qui valide chacune une valeur différente comme la longueur d'une chaine de caractères etc....

public function titre($field,$limit,$error) { if(grapheme_strlen($field) < $limit){ return $this->errors([$error]); } } // ou alors public function Validtitre($flied){ // return blabla }

Et dans ma vue j'appel Validator de cette manière $validator->post(); et cette fonction est validé ou non a l'interieur de la class Validator il faudrai qu'elle prennent plusieurs params en fonction de ce que j'ai a traité.

// c'est un exemple public function post(???){ if(???){ $match = $this->router->matchRoute(); //checkCsrf(); $id = (int) $match['params']['id']; $content = strip_tags(trim($_POST['f_topic_content'])); //$userid = (int) $_SESSION['auth']->id; $this->Content($content,100,'Votre topic dois contenir au moins 100 caractères'); $this->Validtitre($titre,20,'test'); } }

Et ici $this->Content() ou validetitre renvois l'erreur dans public function errors(array $errors = []) qui lui même devra rendre tout le tableau d'erreur dans public function checkError() via la var this->error

//pas de paramêttre c'est important puisse qu'elle est dans ma vue cette fonction //sauf une div par exemple avec des valeurs par défaut public function checkError($div1 = "notify notify-rouge",$div2 = "notify-box-content") { return !empty($this->error) ? "<div class='$div1'><div class='$div2'><ul>".$this->error."</ul></div></div>" : '' ; }

Et ici tu me dit d'utilisé array_push a la place du foreach mais array_push retourne un int et me dit que this->error n'est pas définie, et donc a quel moment je peut init cette variable erreur ? en restant dans la class Validator.

elle existe bien dans mon var_dump j'ai mes 2 tableau sauf que j'ai qu'un k = 0 tout le but est de retourné tout le tableau d'erreur dans checkerror

Avant le réfactoring je faisait ça l'erreur été traité comme ça et la boucle fonctionné grace au .= mais j'imagine qu'on peut pas faire ça avec this->error.

//séparé dans un fichier fonction function errors($messages = []){ if(!empty($messages)){ foreach($messages as $v){ $return = '<li>'.$v.'</li>'; } } return $return; } function CheckErreor($error){ $check = !empty($error) ? "<div class='notify notify-rouge'><div class='notify-box-content'><ul>".$error."</ul></div></div>" : '' ; return $check; } //traitement du formulaire if isset $_POST //etc... $error = ''; if(grapheme_strlen($content) < 100){ $error .= errors(['Votre topic dois contenir au moins 100 caractères']); }if(empty error)...

Et donc c'est cette manière de traité les erreurs qui me semble plus logique après ça peut être une class static mais peut importe le but est qu'il n'y ai pas de paramêtre dans le constructeur et je me demande seulement si c'est possible de fonctionné comme ça ? car le code fonctionne mais oui le foreach retourne qu'une seule erreur.

JoolsMcFly, il y a 3 ans

J'avoue ne pas avor tout lu attentivement mais tu peux faire la validation dans Form :

<?php class Form { private const TITLE_MIN_LENGTH = 30; private const CONTENT_MIN_LENGTH = 100; private string $title; private string $content; private array $errors = []; public function __construct(array $data) { $this->title = $this->sanitizeString($data['title'] ?? ''); $this->content = $this->sanitizeString($data['content'] ?? ''); $this->validateInput(); } private function sanitizeString(string $input): string { return strip_tags(trim($input); // à toi d'ajouter d'autres truc qui te semble nécessaire } public function validateInput() { if (mb_strlen($this->title) < 30) { $this->errors[] = sprintf('Le titre est trop court (min: %s).', self::TITLE_MIN_LENGTH); } if (mb_strlen($this->content) < self::CONTENT_MIN_LENGTH) { $this->errors[] = sprintf('Le titre est trop court (min: %s).', self::CONTENT_MIN_LENGTH); } } public function isValid(): bool { return empty($this->errors); } public function getErrors(): array { return $this->errors; } }

Et dans ton script tu fais

$form = new Form($_POST); if (!$form->isValid()) { ?> <div class="notify notify-rouge"> <div class="notify-box-content"> <ul> <?= "<li>".implode("</li><li>", $form->getErrors())."</li>";?> </ul> </div> </div> <?php }
JoolsMcFly, il y a 3 ans

Salut,

c'est parce que tu fais ça :

$this->errors = ["Les champs sont différents"];

au lieu de

$this->errors[] = "Les champs sont différents";

Dans le premier cas le code dit : "le tableau errors est le suivant : ["Les champ sont différent"].
Dans le 2ème cas le code dit "ajoute Les champs sont différents au tableau errors.

Après il faudrait nommer tes fonctions différemment car required, isDifferent, fileExist devrait renvoyer un booléen et l'utilisation serait du genre

if (!$this->fileExists($path)) { $this->errors[] = "Le fichier n'existe pas."; }

Et je pense que tu ne devrais pas avoir à appeler les méthodes du validateur :

$validator = $this->getValidator() ->notEmpty($pass,$password_confirm) ->isDifferent($pass,$password_confirm) ->validMdp($pass); if($validator->isValid()){

Il faudrait initialiser le validateur avec le champ à vérifier (puisque tu as séparé la maj de mdp de celle de l'email) :

$validator = new PasswordValidator($pass); if ($validator->isValid()) { // maj en BDD } else { // tu passes $validator->getErrors() à la vue }

Et PasswordValidator :

class PasswordValidator { private array $errors = []; private string $pass; public function __construct(string $pass) { $this->pass = $pass; $this->validate(); } private function validate(): void { if (!preg_match('/^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$ %^&*-]).{8,50}$/', $this->pass)) { $this->errors[] = "Le mot de passe doit être composé de 8|50 caractères, de minuscules, une majuscule, de chiffres et d’au moins un caractère spécial."; } } public function isValid(): bool { return empty($this->errors); } public function getErrors(): array { return $this->errors; } }
neecride, il y a 3 ans

bonsoir,

Ha oui ok avant je faisait $error .= errors(["Le slug n'est pas valide admin|modo|membre seulement"]); j'avais pas compris je faisait ça $this->errors[] = ["le truc bidule"] alors ça bugger, donc j'imagine que je pourrai même prendre la clé de $_POST['password'] et faire un truc plus général en randant l'erreur genre le champs $key n'est pas valid mais je veut en faire trop je crois.

Dernière question tu pense que php di peut renvoyer l'erreur dans mes vue sans faire l'initialisation dans ma vue <?= $validator->checkError(); ?> depuis je suis passer a php v8.2 ça m'ouvre plus de porte.

bonne soirée.