Porté des variables réfactoring

Hello :)

Alors pour commencer tu ne récupères pas le résultat de GetRoutes(), du coup à la fin de ta fonction tu peux virer le return et simplement mettre $this->match = $this->router->match();

Ensuite tu as un problème, ton GetRoute concrètement il ne va rien faire :/

// La tu fait un map des routes et tu assignes le match dans $this->match
public function GetRoute(){
        //get|post -> url -> file -> name
        $this->router->map('GET|POST', '/', 'home');
        $this->router->map('GET|POST', '/home', 'home');
        return $this->match = $this->router->match();
}

public function RenderPage(){ 

        $this->GetRoute();

        // La tu ignores ce qu'à enregistré GetRoute ou relancer un match dans $this->router
        $router = $this->router;
        $match = $router->match();

        // Faudrait faire ça à la place: $match = $this->match;
        // Pour utiliser le match initialisé par GetRoute()

        if(is_array($match)){
            if(is_callable($match['target'])){
                    call_user_func_array($match['target'], $match['params']);
            }else{
                '...'
            }
        } else {
                App::setFlash('message','rouge');
                App::redirect('error');
            }
        }
}

En gros j'aurais fais comme ça:

public function renderPage(): void { 
        $this->getRoute();
        $this->redirectIfMatchAreNotArray();
        $this->handleRoute();
}

protected function getRoute(): void {
        $this->router->map('GET|POST', '/', 'home');
        $this->router->map('GET|POST', '/home', 'home');

        $this->match = $this->router->match();
}

protected redirectIfMatchAreNotArray(): void {
        if (!is_array($this->match)) {
                 App::setFlash('message','red');
                 App::redirect('error');
        }
}

protected function handleRoute(): void {
        is_callable($this->match['target'])
                ? $this->callRoute()
                : $this->fillLayoutIfMatchAreNotCallable();
}

protected function callRoute() {
        call_user_func_array($this->match['target'], $this->match['params'])
}

protected function fillLayoutIfMatchAreNotCallable() {
        $params = $thismatch['params'];

        $themeForLayout = $GetParams->themeForLayout($GetParams->GetParam(3));

        ob_start();
        require 'page';
        $contentForLayout = ob_get_clean();

        require 'layout';
}

Alors je l'ai fais à l'aveugle, alors pas sur que ça fonctionne mais dans l'idée c'est ça :p

En gros tu as l'erreur cannot redeclare route parce que tu déclares 2 fois un router, une fois avec les routes en utilisant GetRoute, et une autre fois un router vide sans les routes avec $match = $router->match();

Sinon pour la structure, pas la peine de faire des petits fichiers, c'est mieux de découper le code en pleins de petites functions, histoire de rendre le tout lisible, une class PHP doit raconter une histoire, une function public principal renderPage() qui raconte cette histoire, et des functions protected qui executent les actions :p

Ensuite pour le nom des functions, tu as deux écoles:

• Soit tu fait des nom de functions à rallonge pour expliquer ce que ça fait (un peu comme j'ai fais)
• Soit tu fait des commentaires, mais les commentaires devront être présents au dessus de TOUTES les functions:

/**
  * Render the page.
  */
public function renderPage(): void { 
        $this->getRoute();
        $this->redirectError();
        $this->handleRoute();
}

/**
  * Get available routes.
  */
protected function getRoute(): void {
        $this->router->map('GET|POST', '/', 'home');
        $this->router->map('GET|POST', '/home', 'home');

        $this->match = $this->router->match();
}

/**
  * Redirect to error page if match are not array.
  */
protected redirectError(): void {
        if (!is_array($this->match)) {
                 App::setFlash('message','red');
                 App::redirect('error');
        }
}

/**
  * Handle routes.
  */
protected function handleRoute(): void {
        is_callable($this->match['target'])
                ? $this->callRoute()
                : $this->fillLayout();
}

/**
  * Call the route.
  */
protected function callRoute() {
        call_user_func_array($this->match['target'], $this->match['params'])
}

/**
  * Fill layout if match are not callable.
  */
protected function fillLayout() {
        $params = $thismatch['params'];

        $themeForLayout = $GetParams->themeForLayout($GetParams->GetParam(3));

        ob_start();
        require 'page';
        $contentForLayout = ob_get_clean();

        require 'layout';
}

Bonsoir popotte, je vais reprendre ton idée et faire des testes, faudra que je creuse un peut plus car cette class devra être utilisé dans d'autres comme le GET csrf qui n'est pas injecté avec $_GET pour l'instant je fait ma fouine avec des global.

Bonne soirée.

Les global c'est pas top si tu veux fonctionner en class (tu peux le faire, c'est pas une erreur de le faire, mais c'est pas le top du top), utilises plutot l'injection de dependances pour injecter cette class dans les autres class qui ont besoin de l'utiliser :p

En gros tu as un dossier libs ou serviceset tu places cette class dans ce dossier

Ensuite dans les class qui devront utiliser ce service tu a juste a faire:

use Services\TonService; // Enfin le nom de ta class réutilisable

class Random {
    protected $tonService;

    public function __construct(TonService $tonService) {
        $this->tonService = $tonService;
    }
}

Alors tu es en PHP natif, alors pour faire ça tu devra utiliser php-di/php-di https://php-di.org/ la doc -> https://php-di.org/doc/getting-started.html

Renseignes toi sur l'injection de dépendances et les singletons ;)

Au fait si tu utilises du js dans ton projet, essayes ça https://esbuild.github.io/ c'est pas mal :p

Justement l'injection de dépendance c'est sur ce sujet que je boss, pour te donner un bout.

namespace App;
use \AltoRouter;

class Route {

    public $router;
    public $match;

    public function __construct()
    {
        $this->router = new AltoRouter();
        $this->match = $this->router->match();
    }
}

Et du coup ce que j'ai fais pour résoudre le redéclare route ; déjà j'ai viré le return comme tu le préconisé sur la classe GetRoute et déjà ça va mieux, donc j'ai fais ceci

public static function AltoMatch(){
      return new Route();
}

public function Target(){
    $this->GetRoute();
    return $this->router->match();
 }      

du coup mon checkCsrf deviens (les noms c'est pas glorieux je sais lol)

//j'appel mes paramêtres comme ça
//Route::AltoMatch()->Target()['params']['...']

    public static function checkCsrf(){

        if(
            (isset($_POST['csrf']) && $_POST['csrf'] == self::csrf()) 
            ||
            (isset(Route::AltoMatch()->Target()['params']['getcsrf']) && Route::AltoMatch()->Target()['params']['getcsrf'] == self::csrf())
          )
        {
          return true;  
        }
        App::setFlash('<strong>Oh oh!</strong> C\'est pas bien ! <strong> :( Faille CSRF </strong>','rouge');
        App::redirect('error');

    }

Et donc ça fonctionne, si faire un static et rappeler GetRoute a chaque fois ne pose pas de soucis lol

J'ai par exemple fais un bouton Up du coup je peut récupéré mes paramêtres de la même façon mais ça implique de rapeller GetRoute mais cette fois sans l'erreur redéclare route.

//j'appel mes paramêtres comme ça
//Route::AltoMatch()->Target()['params']['...']

    public function Up(){

        if(isset(Route::AltoMatch()->Target()['params']['up'])){

            Session::checkCsrf();//on verifie les faille csrf

            $id = (int) Route::AltoMatch()->Target()['params']['up'];
            $this->Cnx()->Update("UPDATE f_topics SET f_topic_date = NOW() WHERE id = ?",[$id]);
            App::setFlash('Votre topic a bien été remonter');
            App::redirect('topic-'.$id);
        }

    }

Bonne soirée.

Ps : global je l'utilise parce que j'ai pas fini de tout faire j'ai encore des script php brut.

Ah d'accord, alors pour l'injection, le principe c'est de passer par l'injection vraiment partout, même la class Route, par ex:

namespace App;
use \AltoRouter;

class Route {
    public $router;
    public $match;

    public function __construct(AltoRouter $router) // <- injection de dépendance :p
    {
        $this->router = $router;
        $this->match = $this->router->match();
    }
}

Ensuite il faudrait remplacer Route::AltoMatch par une instance de Route construite et utiliser router $route = new Route; $route->router;

L'injection de dépendance ça sert à éviter l'utilisation du static :p

Pour le checkCSRF les noms sont bien xD Par contre il faudrait qu'il soit dans une class à part, une class, un role :p

Par contre il faudrait éviter le static et utiliser... Une injection de dépendance :p Mmmmh par contre ça va être un peu compliqué... Je tente un truc si ça marche pas au moins tu vois à peu prêt dans l'idée ce qu'il faut faire ^^' Et j'en ai profité pour pousser un peu dans le code ^^

// Le controller qui gère tout ce qui est Topic
class TopicController {
    protected Route $route;
    protected Session $session;

    puublic function__construct(Route $route, Session $session) {
        $this->route = $route;
        $this->session = $session;
    }

    public function Up() {
        if(isset($this->route->Target()['params']['up'])){
            try {
                $this->session->checkCSRF(); // on verifie les faille csrf
            } catch (FailleCSRFException $e) {
                App::setFlash($e->getMessage(),'rouge');
                App::redirect('error');
            }

            $id = (int) $this->route->Target()['params']['up'];
            $this->Cnx()->Update("UPDATE f_topics SET f_topic_date = NOW() WHERE id = ?",[$id]);

            App::setFlash('Votre topic a bien été remonter');
            App::redirect('topic-'.$id);
        }
    }
}

// Le check CSRF
class Session {
    protected Route $route;

    public function __construct(Route $route) {
        $this->route = $route;
    }

    public static function checkCSRF(Route $route){
        if(!$this->checkPostCSRF() && !this->checkRouteCSRF()) {
            throw new FailleCSRFException();
        }

        return true;
    }

    protected function checkPostCSRF() {
        return isset($_POST['csrf']) && $_POST['csrf'] == self::csrf();
    }

    protected function checkRouteCSRF() {
        return isset($this->route->Target()['params']['getcsrf']) && $this->route->Target()['params']['getcsrf'] == self::csrf();
    }
}

// L'exception pour la faille CSRF
class FailleCSRFException {
    public function getMessage() {
        return '<strong>Oh oh!</strong> C\'est pas bien ! <strong> :( Faille CSRF </strong>';
    }
}

Après l'utilisation du static c'est une possibilité, mais si tu bosse sur l'injection c'est mieux d'utiliser ça, eeet l'injection c'est mieux :p

Ensuite si vraiment tu as besoin d'une nouvelle instance vide de Route (et donc sans les target):

public static function AltoMatch(){
      return new Route();
}

Mieux vaut l'appeler en dehors de l'instance existante de route et donc en dehors de la class:

$route2 = new Route();

Top alors si le global c'est juste temporaire :p Tu peux en faire pour définir des helpers, et je crois que c'est le seul cas où tu peux placer en global, par contre je crois que composer peut gérer le chargement des helpers :p

Après il y a pas mal de choses qui peuvent être améliorés mais déjà comme ça ce sera pas mal opti :p

Bonsoir,

J'ai déjà des class distinct pour mes session etc... j'ai créer une class faite pour ça ou j'y met mon inpuCsfr, checkSessionCsrf etc...

il est vrai que j'ai remarquer que le static c'est pas top mais pour moi c'était pratique ça evite justement de faire des instances lol, mais c'est des class que j'ai récupéré d'il y a 2 ans quand je tester l'objet j'avais la flemme de tous refaire lol

du coup ma class qui gère les topic ça donne ça

    /*
    * return instance Parameters
    */
    private function Params(){
        return new Parameters();
    }

    /*
    * return instance PDO
    */
    private function Cnx(){
        return new Database();
    }   

    /*
    * return instance des route
    */  
    private function AltoMatch(){
        return new Route();
    }

    /*
    * return instance de session
    */
    private function Session(){
        return new Session();
    }

Que j'ai sobrement appeler "class ViewTopic" il n'y a que la class pagination ou je galère elle fonctionne quand j'ai pas de jointure, alors j'ai pour le moment fais une pagination sur tout mes controller Forum, ViewTopic, ViewForum etc... mais là je fera ça plus tard au pire il n'y a que le CountData pour faire le pagetotal qui coince.

Ce que je ne comprend pas par contre c'est le constructeur

public function __construct(AltoRouter $router)

Quand je fais une instance de Route il me demande un paramêtre et je sais pas quoi lui donner lol

En tout cas ça fais moins machine a gaz !

Ahah oui par contre si tu refacto tout ton code pour en plus implémenter en injection de dépendance c'et sur que tu devra te battre contre la fleme parce que c'est du boulot xD

Alors pour ton ViewTopic, faut pas faire de function qui retourne des instances de telle ou telle class, ces class tu les créées en dehors de ViewTopic, et si tu en a besoin dans la class ViewTopic tu injectes :p

Ensuite oui pour cette partie:

public function __construct(AltoRouter $router)

Alors a la base tu faisais ça:

public function __construct()
    {
        $this->router = new AltoRouter();
        $this->match = $this->router->match();
    }

Donc en gros tu doit wire la class:

$altoRouter = new AltoRouter();
$route = new Route($altorouter);

Ensuite ta class ViewTopic, alors toutes tes class qui retournent des nouvelles instances... faut les enlever et instancier directement dans l'index :p

$altoRouter = new AltoRouter();
$parameters = new Parameters();
$database = new Database();
$session = new Session();
$route = new Route($altoRouter);
$viewTopic = new ViewTopic($parameters, $database, $session, $route);

class Route {
    public function __construct(AltoRouter $altoRouter)
}

class ViewTopic {
    protected $parameters;
    protected $database;
    $protected $session;
    $protected $route;

    public function __construct($parameters, $database, $session, $route) {
        $this->parameters = $parameters;
        $this->database = $database;
        $this->session = $session;
        $this->route = $route;
    }
}

Déjà c'est un bon début, mais c'est pas top, faudra te renseigner sur les traits pour optimiser tout ça, après tu pourras faire un truc du genre:

class ViewTopic {
    use Parameters;
    use Database;
    use Session;
    use Route;
}

Au niveau des functions de ViewTopic, déjà la nom de ta class devrait être TopicsManager ou TopicsController, ensuite il ne devrait y avoir que ces functions:

public function index() {} // Lister tous les topics

public function show(Topic $topic) {} // Page d'un topic, en parametre il y a un modèle Topic

public function create() { //page de création d'un topic
    $topic = new Topic;
    ...
}

public function store() {} // Creation d'un nouveau topic

public function edit(Topic $topic) {} // Page d'édition d'un topic

public function update(Topic $topic) {} // Update d'un topic

public function delete(Topic $topic) {} // Suppression d'un topic

Comme ça tu respectes l'architecture MVC ^^

Il y a trois principes a respecter pour faire du bon code:

• L'architecture MVC
• Le SOLID
• Les standards PSR "must" et "must not", les standard "should" sont optionnelles mais c'est mieux :p

Au fait t'as un Github de ton projet?

Il y a trois principes a respecter pour faire du bon code:

L'architecture MVC
Le SOLID
Les standards PSR "must" et "must not", les standard "should" sont optionnelles mais c'est mieux :p

J'emets juste une petite réserve sur MVC dans les bons code. Certes c'est mieux de faire du MVC que de ne rien faire du tout :D

Mais MVC n'est pas le seul pattern que l'on peut utiliser. Il y a par exemple ADR (Action Domain Responder qui est un MVC plus orienté pour le web à la base.

On peut aussi partir sur de l'hexagonale architexture quand on a un projet qui va (ou est déjà) conséquant. Sachant que dans cette architecture on y retrouve souvant mais ce n'est pas une obligation, du DDD (Domain Driven Design)

Personnellement depuis que je pratique le DDD et l'hexagonale architecture, je trouve que je m'y retrouve mieux (Bon c'est aussi parceque je peux passer 5 minutes à chercher le nom d'une classe xD).

Pour le reste, SOLID pour éviter du code "STUPID" je suis d'accord et les standard PSR aussi, il faut au moins connaitre les PSR : 1, 12 et 4 au minimum)

PS: Voici l'architecture des dossiers que l'on pourrait voir : https://pastebin.com/0Ed7nQfw

Oui je suis resté focus sur le MVC parce que quand on débute en PHP faut commencer par ça car c'est le plus basique et le plus répandu, les autres architectures c'est bien de les voir plus tard mais MVC ça reste l'architecture a avoir en tant que première fois ^^

Pour l'architecture que tu montres, alors c'est très bien, mais pour un débutant je pens eque commencer par du MVC ça reste mieux, c'est ce qui est le plus répandu x)

Attention mon message n'est pas de dire qu'il faut faire du DDD ou hexagonal architecture car c'est vraiment lié à un besoin un peu plus conséquents. Je voulais juste soulligner le "pour faire du bon bon code il faut faire du mvc" qui la pour le coup n'est pas vrai.

Peut-être que c'est moi qui est mal compris ta phrase de départ mais je voulais juste remettre au claire :)

D'ailleurs pour commencer, on peut partir sur ADR qui est un MVC un peu différent mais qui reprends les même principes

Model => Domain
View => Responder
Controller => Action

Ah oui je m'étais pas bien exprimé, en gros je voulais dire que pour du bon code il fallait pas faire une architecture soit même mais suivre une architecture standard, et j'ai pensé au MVC ^^'

Alors l'ADR disons que c'est une architecture abordable, mais pour commencer ça reste quand même le MVC qui est mieux, les tutos débutants sont tous en MVC, les entreprises fonctionnent en majorité en MVC, bref c'est le top pour commencer

Ensuite une fois que c'est acquis j'ai une préférence pour le DDD quand même, à la base je l'utilisais pour organiser Golang et finalement c'est pas mal du tout ^^