Refactoring procedural vers poo

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

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.

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

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.

Bonjour,

Je me dois quand même de faire une réponse pour la postérité lol

Je suis parvenue a faire ceci

https://github.com/neecride/Forum-Project/blob/main/src/App/Validator.php
https://github.com/neecride/Forum-Project/blob/main/src/Action/TopicAction.php

Mon validateur fonctionne si ce n'est qu'il affiche mes erreurs une par une et pas toutes les erreurs que je catch, je suis toujours a la recherche d'un mieux.

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

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.