IdentifiantMot de passe
Loading...
Mot de passe oublié ?Je m'inscris ! (gratuit)
Navigation

Inscrivez-vous gratuitement
pour pouvoir participer, suivre les réponses en temps réel, voter pour les messages, poser vos propres questions et recevoir la newsletter

Langage PHP Discussion :

Votre avis sur mon code, syntaxe, propreté et code commenté


Sujet :

Langage PHP

Vue hybride

Message précédent Message précédent   Message suivant Message suivant
  1. #1
    Membre éclairé
    Homme Profil pro
    Webmaster
    Inscrit en
    Juillet 2015
    Messages
    518
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 38
    Localisation : France, Hauts de Seine (Île de France)

    Informations professionnelles :
    Activité : Webmaster

    Informations forums :
    Inscription : Juillet 2015
    Messages : 518
    Par défaut Votre avis sur mon code, syntaxe, propreté et code commenté
    Bonjour,

    Je suis en train de faire un forum pour ma formation, mon forum fonctionne bien mais j'ai besoin de vos avis sur mon code, propreté, code commenté correctement, syntaxe.. etc (bien-sur je n'utilise pas le MVC ni de POO juste du php basic pour le moment..)

    (Je compte distribuer mon forum en libre téléchargement dans quelques jours)

    Je mets uniquement une page pour le moment, histoire d'avoir vos avis progressivement :
    Note : Je suis conscient que vous n'avez pas le reste des pages pour le moment, ni la structure des tables mais je ne demande pas un jugement poussé, simplement savoir si mon code est lisible par le plus grand nombre.

    voici la page de création de topic, nouveau.php

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
    11
    12
    13
    14
    15
    16
    17
    18
    19
    20
    21
    22
    23
    24
    25
    26
    27
    28
    29
    30
    31
    32
    33
    34
    35
    36
    37
    38
    39
    40
    41
    42
    43
    44
    45
    46
    47
    48
    49
    50
    51
    52
    53
    54
    55
    56
    57
    58
    59
    60
    61
    62
    63
    64
    65
    66
    67
    68
    69
    <?php
    require '../../inc/functions.php';
    logged_only();
    require_once '../../inc/db.php';
     
    $idsalon = (int) $_GET['idsalon'];
     
    if(!empty($_POST)){
     
        if(empty($_POST['sujet'])){
            $_SESSION['flash']['danger'] = 'Veuillez renseigner un sujet';
        }
     
        if(empty($_POST['message'])){
            $_SESSION['flash']['danger'] = 'Votre message est trop court';
        }
     
        if(!isset($_SESSION['flash'])){
            // On enregistre le sujet dans la table forum_sujets
            $req = $pdo->prepare("INSERT INTO forum_sujets SET sujet = ?, idsalon = ?, idmbr = ?, idmbr_dernier = ?, derniermsg = NOW(), etat = 1");
            $sujet = htmlspecialchars($_POST['sujet']);
            $user_id = $_SESSION['auth']->id;
            $req->execute([$sujet, $idsalon, $user_id, $user_id]);
            $idsujet = $pdo->lastInsertId();
            $req->closeCursor(); // On libère le curseur pour la prochaine requête
     
            // On enregistre le message dans la table forum_messages
            $req = $pdo->prepare("INSERT INTO forum_messages SET idsujet = ?, message = ?, idmbr = ?, dateheure = NOW()");
            $message = trim(htmlspecialchars(stripslashes($_POST['message'])));
            $req->execute([$idsujet, $message, $user_id]);
            $req->closeCursor(); // On libère le curseur pour la prochaine requête
     
            // table forum_profils
            $req = $pdo->query('SELECT * FROM forum_profils WHERE idmbr = '.$user_id.'');
            $user = $req->fetch();
     
            // On vérifie si le membre a déjà posté un message sur le forum pour pouvoir choisir entre update ou insert
            if($user){
                // Si un enregistrement a ete trouvé alors on update le nombre de messages postés par le membre
                $nbmsg_new = ++$user->nbmsg; // on ajoute 1 au nombre de messages postés
                $pdo->exec('UPDATE forum_profils SET nbmsg = '.$nbmsg_new.' WHERE idmbr = '.$user_id.'');
            }else{
                // Si c'est le premier message du membre alors on utilise insert
                $pdo->exec('INSERT INTO forum_profils (idmbr, nbmsg) VALUES('.$user_id.', 1)');
            }
     
            $_SESSION['flash']['success'] = 'Votre message a bien été envoyé';
            header('Location: sujet.php?idsujet='.$idsujet.'');
            exit();
        }
    }
     
    require '../../inc/header.php';
    ?>
        <h1>Nouveau sujet</h1>
     
        <form action="" method="POST">
            <div class="form-group">
                <label for="sujet">Sujet</label>
                <input type="text" name="sujet" class="form-control" id="sujet" placeholder="Mon sujet...">
            </div>
            <div class="form-group">
                <label for="message">Votre message</label>
                <textarea name="message" class="form-control" id="message" rows="6"></textarea>
            </div>
            <button type="submit" class="btn btn-primary">Envoyer</button>
        </form>
     
    <?php require '../../inc/footer.php'; ?>
    MERCI a vous

  2. #2
    Expert confirmé
    Avatar de rawsrc
    Homme Profil pro
    Dev indep
    Inscrit en
    Mars 2004
    Messages
    6 142
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 49
    Localisation : France, Bouches du Rhône (Provence Alpes Côte d'Azur)

    Informations professionnelles :
    Activité : Dev indep

    Informations forums :
    Inscription : Mars 2004
    Messages : 6 142
    Billets dans le blog
    12
    Par défaut
    Salut,

    allez c'est parti :
    - rien ne te garantit que $_GET['idsalon'] existe
    - aère le code : ajoute des espaces entre les mots clés du genre if, else...
    - ne commente pas les évidences du style :
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    // On enregistre le sujet dans la table forum_sujets
    $req = $pdo->prepare("INSERT INTO forum_sujets SET sujet = ?, idsalon = ?, idmbr = ?, idmbr_dernier = ?, derniermsg = NOW(), etat = 1");
    On comprend en lisant INSERT INTO que tu t'apprêtes à perister les données en table
    - tu utilises PDO et tu fais $sujet = htmlspecialchars($_POST['sujet']); pour échapper les données ?!!!???
    On ne mélange surtout pas les échappements : ceux pour la base de données n'ont rien à voir avec ceux pour l'affichage.
    Sans compter que si demain tu souhaites attaquer ta base en direct c'est-à-dire autrement qu'avec ton site, tu vas être bien avancé pour retrouver les échappements HTML dans tes requêtes...
    - ne te préoccupe pas de la libération du curseur PDO, PHP le fait pour toi et de manière bien plus performante
    - ne mélange pas les langues : soit tu nommes les variables en anglais (ma préférence, et de loin) soit tu le fais en français mais j'abhorre le mélange des deux
    - Tiens pour exemple, quand tu fais $req = $pdo->prepare, la doc te dit que cela renvoie un statement, alors pourquoi ne pas directement identifier correctement cette info : $stmt = $pdo->prepare ?
    - Là c'est l'Everest : $message = trim(htmlspecialchars(stripslashes($_POST['message'])));, bonjour l'empilement ! Avec ça, tu pètes les chaînes UTF-8.
    - Tu ne renvoie pas les données à l'utilisateur quand le formulaire a été rempli mais contient des erreurs ? Il doit tout se retaper jusqu'à ce que ça passe ?

    Bonne chance

  3. #3
    Membre éclairé
    Homme Profil pro
    Webmaster
    Inscrit en
    Juillet 2015
    Messages
    518
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 38
    Localisation : France, Hauts de Seine (Île de France)

    Informations professionnelles :
    Activité : Webmaster

    Informations forums :
    Inscription : Juillet 2015
    Messages : 518
    Par défaut
    Merci pour ta réponse, voici la correction :

    Citation Envoyé par rawsrc Voir le message
    - rien ne te garantit que $_GET['idsalon'] existe
    Fait.

    Citation Envoyé par rawsrc Voir le message
    - aère le code : ajoute des espaces entre les mots clés du genre if, else...
    Fait.

    Citation Envoyé par rawsrc Voir le message
    - ne commente pas les évidences du style :
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    // On enregistre le sujet dans la table forum_sujets
    $req = $pdo->prepare("INSERT INTO forum_sujets SET sujet = ?, idsalon = ?, idmbr = ?, idmbr_dernier = ?, derniermsg = NOW(), etat = 1");
    On comprend en lisant INSERT INTO que tu t'apprêtes à perister les données en table
    Rectifié.

    Citation Envoyé par rawsrc Voir le message
    - tu utilises PDO et tu fais $sujet = htmlspecialchars($_POST['sujet']); pour échapper les données ?!!!???
    On ne mélange surtout pas les échappements : ceux pour la base de données n'ont rien à voir avec ceux pour l'affichage.
    Sans compter que si demain tu souhaites attaquer ta base en direct c'est-à-dire autrement qu'avec ton site, tu vas être bien avancé pour retrouver les échappements HTML dans tes requêtes...
    Fait.

    Citation Envoyé par rawsrc Voir le message
    - ne te préoccupe pas de la libération du curseur PDO, PHP le fait pour toi et de manière bien plus performante
    Supprimé.

    Citation Envoyé par rawsrc Voir le message
    - ne mélange pas les langues : soit tu nommes les variables en anglais (ma préférence, et de loin) soit tu le fais en français mais j'abhorre le mélange des deux
    Oui effectivement, je vais renommer les noms, champs et variables du script.

    Citation Envoyé par rawsrc Voir le message
    - Tiens pour exemple, quand tu fais $req = $pdo->prepare, la doc te dit que cela renvoie un statement, alors pourquoi ne pas directement identifier correctement cette info : $stmt = $pdo->prepare ?
    Oui bonne idée je vais renommer les noms d'ici quelques jours.

    Citation Envoyé par rawsrc Voir le message
    - Là c'est l'Everest : $message = trim(htmlspecialchars(stripslashes($_POST['message'])));, bonjour l'empilement ! Avec ça, tu pètes les chaînes UTF-8.
    oui mais que faire ?

    Citation Envoyé par rawsrc Voir le message
    - Tu ne renvoie pas les données à l'utilisateur quand le formulaire a été rempli mais contient des erreurs ? Il doit tout se retaper jusqu'à ce que ça passe ?
    Fait.

    Vl'a ma nouvelle copie m'sieur :

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
    11
    12
    13
    14
    15
    16
    17
    18
    19
    20
    21
    22
    23
    24
    25
    26
    27
    28
    29
    30
    31
    32
    33
    34
    35
    36
    37
    38
    39
    40
    41
    42
    43
    44
    45
    46
    47
    48
    49
    50
    51
    52
    53
    54
    55
    56
    57
    58
    59
    60
    61
    62
    63
    64
    65
    66
    67
    68
    69
    70
    71
    72
    73
    74
    75
    <?php
    require '../../inc/functions.php';
    logged_only();
    require_once '../../inc/db.php';
     
    $idsalon = (int) $_GET['idsalon'];
     
    // On vérifie que le salon existe bien.
    // Inutile de faire une requete préparé ? (oui c'est bien une question), je sécurise déjà la variable grâce a (int) plus haut...
    $req = $pdo->query('SELECT * FROM forum_salons WHERE idsalon = '.$idsalon.'');
    $verif_salon = $req->fetch();
    if (!$verif_salon) {
        $_SESSION['flash']['danger'] = 'Ce salon n\'existe pas';
        header('Location: /membres/forum/index.php');
        exit();
    }
     
    if (!empty($_POST)) {
     
        if (empty($_POST['sujet'])) {
            $_SESSION['flash']['danger'] = 'Veuillez renseigner un sujet';
        }
     
        if (empty($_POST['message'])) {
            $_SESSION['flash']['danger'] = 'Votre message est trop court';
        }
     
        if (!isset($_SESSION['flash'])) {
            // Le sujet et le message sont bien remplis on peut donc continuer
            // On commence par enregistrer le sujet dans la table forum_sujets
            $req = $pdo->prepare("INSERT INTO forum_sujets SET sujet = ?, idsalon = ?, idmbr = ?, idmbr_dernier = ?, derniermsg = NOW(), etat = 1");
            $user_id = $_SESSION['auth']->id;
            $req->execute([$_POST['sujet'], $idsalon, $user_id, $user_id]);
            $idsujet = $pdo->lastInsertId();
     
            // Ensuite on enregistre le message dans la table forum_messages
            $req = $pdo->prepare("INSERT INTO forum_messages SET idsujet = ?, message = ?, idmbr = ?, dateheure = NOW()");
            $message = trim(htmlspecialchars(stripslashes($_POST['message'])));
            $req->execute([$idsujet, $message, $user_id]);
     
            // On vérifie si le membre a déjà posté un message sur le forum pour pouvoir choisir entre update ou insert
            $req = $pdo->query('SELECT * FROM forum_profils WHERE idmbr = '.$user_id.'');
            $user = $req->fetch();
            if ($user) {
                // Si un enregistrement a ete trouvé alors on update le nombre de messages postés par le membre
                $nbmsg_new = ++$user->nbmsg; // on ajoute 1 au nombre de messages postés
                $pdo->exec('UPDATE forum_profils SET nbmsg = '.$nbmsg_new.' WHERE idmbr = '.$user_id.'');
            } else {
                // Si c'est le premier message du membre alors on utilise insert
                $pdo->exec('INSERT INTO forum_profils (idmbr, nbmsg) VALUES('.$user_id.', 1)');
            }
     
            $_SESSION['flash']['success'] = 'Votre message a bien été envoyé';
            header('Location: sujet.php?idsujet='.$idsujet.'');
            exit();
        }
    }
     
    require '../../inc/header.php';
    ?>
        <h1>Nouveau sujet</h1>
     
        <form action="" method="POST">
            <div class="form-group">
                <label for="sujet">Sujet</label>
                <input type="text" name="sujet" value="<?= empty($_POST['sujet']) ? '' : htmlspecialchars($_POST['sujet'], ENT_QUOTES); ?>" class="form-control" id="sujet" placeholder="Mon sujet...">
            </div>
            <div class="form-group">
                <label for="message">Votre message</label>
                <textarea name="message" class="form-control" id="message" rows="6"><?= empty($_POST['message']) ? '' : htmlspecialchars($_POST['message'], ENT_QUOTES); ?></textarea>
            </div>
            <button type="submit" class="btn btn-primary">Envoyer</button>
        </form>
     
    <?php require '../../inc/footer.php'; ?>
    Merci

  4. #4
    Modérateur
    Avatar de sabotage
    Homme Profil pro
    Inscrit en
    Juillet 2005
    Messages
    29 208
    Détails du profil
    Informations personnelles :
    Sexe : Homme

    Informations forums :
    Inscription : Juillet 2005
    Messages : 29 208
    Par défaut
    oui mais que faire ?
    La vraie question c'est plutôt : que souhaites-tu faire ?
    Quelle était le but de ces 3 fonctions ?

    Si tu ne sais pas toi même alors peut-être que ça ne sert à rien

    Bon d’expérience on trouve ce genre de choses dans des vieux codes PHP4 mal écrits.
    On préfère utiliser htmlspecialchars() au moment de l'affichage plutôt qu'à l'insertion
    stripslashes() servait il y a plus de 10 ans quand les magic_quotes ajoutaient des slash.
    concernant trim(), je dirai que s'il y a un espace ou autre aux bouts du message saisie ... la belle affaire.
    N'oubliez pas de consulter les FAQ PHP et les cours et tutoriels PHP

  5. #5
    Membre éclairé
    Homme Profil pro
    Webmaster
    Inscrit en
    Juillet 2015
    Messages
    518
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 38
    Localisation : France, Hauts de Seine (Île de France)

    Informations professionnelles :
    Activité : Webmaster

    Informations forums :
    Inscription : Juillet 2015
    Messages : 518
    Par défaut
    bon je viens de supprimer l'everest comme dirait rawsrc

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    // Ensuite on enregistre le message dans la table forum_messages
    $req = $pdo->prepare("INSERT INTO forum_messages SET idsujet = ?, message = ?, idmbr = ?, dateheure = NOW()");
    $req->execute([$idsujet, $_POST['message'], $user_id]);
    voila ! d'autres suggestions ?

+ Répondre à la discussion
Cette discussion est résolue.

Discussions similaires

  1. Réponses: 0
    Dernier message: 25/02/2012, 16h00
  2. Votre avis sur mon code
    Par Schopenhauer dans le forum Fortran
    Réponses: 4
    Dernier message: 04/05/2011, 15h12
  3. [XL-2003] Votre avis sur mon code en VBA ?
    Par [ZiP] dans le forum Macros et VBA Excel
    Réponses: 2
    Dernier message: 02/03/2010, 13h56
  4. Réponses: 3
    Dernier message: 01/09/2008, 14h43
  5. [FFT] Votre avis sur mon code
    Par deubelte dans le forum C++
    Réponses: 1
    Dernier message: 10/02/2007, 20h14

Partager

Partager
  • Envoyer la discussion sur Viadeo
  • Envoyer la discussion sur Twitter
  • Envoyer la discussion sur Google
  • Envoyer la discussion sur Facebook
  • Envoyer la discussion sur Digg
  • Envoyer la discussion sur Delicious
  • Envoyer la discussion sur MySpace
  • Envoyer la discussion sur Yahoo