Propreté d'un code source avec isset()

Besoin de conseils pour des bonnes pratiques ^^

Le problème exposé dans ce sujet a été résolu.

Salut à tous !

Je développe un mini-jeu html, il faut cocher au moins une case d’un formulaire, sans toutes les cocher.

Il y aura plein de cases, et le code php a besoin d’être propre et optimisé si on ne veut pas s’y perdre.

Dans le code actuel, je pense que isset() se retrouve trop de fois et ce sera vite encombré si j’ai une dizaine de cases à vérifier.

Que feriez-vous pour optimiser et/ou améliorer la propreté du code ?
Quelles sont les « bonnes pratiques » ?

Merci d’avance pour les réponses ^^

page.html :

Afficher/Masquer le contenu masqué
<!DOCTYPE html>
<html lang="fr">
<head>

    <meta charset="utf-8">
    <title>page.html</title>

</head>
<body>

    <form action="page2.php" method="post">

        a <input type="checkbox" name="a"><br>
        b <input type="checkbox" name="b"><br>
        c <input type="checkbox" name="c"><br>
        <!-- etc etc... -->
        y <input type="checkbox" name="y"><br>
        z <input type="checkbox" name="z"><br>

        <input type="submit" value=" OK ">

    </form>

</body>
</html>

page2.php :

Afficher/Masquer le contenu masqué
<?php

if (isset($_POST['a'], $_POST['b'], $_POST['c'], $_POST['y'], $_POST['z']))
{
    echo 'Heu... on ne peut pas cocher toutes les cases ^^';
}
else if (
    isset($_POST['a']) || 
    isset($_POST['b']) || 
    isset($_POST['c']) || 
    isset($_POST['y']) || 
    isset($_POST['z'])
)
{
    echo 'C\'est OK ^^';
}
else
{
    echo 'Merci de cocher au moins une des cases ^^';
}

echo '<br><br><a href="page.html">Retour</a>';
+0 -0

Salut !

Utiliser des tableaux et des boucles pour éviter les répétitions.

Pense que tu peux utiliser tout ça pour générer tes cases ET pour traiter le résultat, pas que l’un ou l’autre.

Afficher/Masquer le contenu masqué
<?php
$alphabet = 'abcdefghijklmnopqrstuvwxyz';

for ($i = 0; $i < strlen($alphabet); $i++) {
  echo $alphabet[$i];
}
+0 -0

et pour le coup, un foreach est même meilleurs :

<?php
$lettres_autorises = ['a', 'b', 'c', 'y', 'z'];
$found = false;
$all = true;
foreach($lettres_autorieses as $lettre) {
    if (isset($_POST[$lettre])){
        $found = true;
     }
     else {
        $all = false;
     }

}
if ($found && !$all) {
   echo "OK";
} else {
   echo "BAD";
}

revenons un peu sur le code :

  • le foreach permet de parcourir le tableau des possibilités
  • comme on veut qu’au moins une case soit cochée mais pas toutes, on doit trouver un moyen de découvrir si une est cochée, et un moyen de découvrir si toutes son cochées

Habituellement, je ferai donc deux fonctions une qui dit "si une est cochée" et l’autre qui dit "si toutes sont cochées". Comme ces deux cas sont des cas simples, j’ai néanmoins proposé de "mixer" les deux calculs.

+1 -0

En reprenant le code d’astragis, tu peux aussi compter le nombre d’apparition :

<?php
$lettres_autorisees = ['a', 'b', 'c', 'y', 'z'];
$count = 0;
foreach($lettres_autoriesees as $lettre) {
    if (isset($_POST[$lettre]))
        $count++;
}
if (0 < $count && $count != count($lettres_autorisees)) {
   echo "OK";
} else {
   echo "BAD";
}
+0 -0

En plus comme en php les tableaux c’est magique on peut aller encore plus loin en faisant des ensembles. Mais je pense qu’ici l’idée c’est déjà d’aider @Ludwig à se tourner vers les tableaux, ce sont la structure la plus puissante de php.

Le principal problème du comptage, c’est qu’il n’est pas expressif. Un "joli" code c’est un code qu’on peut lire et auquel on donne le bon sens du premier coup d’oeil, je pense.

<?php

$lettres_autorises = ['a', 'b', 'c', 'y', 'z'];
$post_keys = array_keys($_POST);
$difference = array_diff($lettres_autorises, $post_keys);
$allAreGiven = !(bool)($difference);
$atLeastOneIsGiven = $difference != $lettres_autorises;
+0 -0

Merci pour vos réponses ! :)

C’est vrai, $_POST est en fait un gentil tableau ^^

J’ai écrit un code simplifié :

page2.php :

<?php

// J'ai remarqué que isset($_POST) renvoie toujours true,
// donc il est toujours possible de compter les éléments dans $_POST
$n_cases = count($_POST);

if ($n_cases >= 26)
{
    echo 'Heu... on ne peut pas cocher toutes les cases ^^';
}
else if ($n_cases == 0)
{
    echo 'Merci de cocher au moins une des cases ^^';
}
else
{
    echo 'C\'est OK ^^';
}

echo '<br><br><a href="page.html">Retour</a>';
+0 -0

Merci pour la remarque ^^

Je tente avec ceci :

page2.php :

Afficher/Masquer le contenu masqué
<?php

// J'ai remarqué que isset($_POST) renvoie toujours true,
// donc il est toujours possible de compter les éléments dans $_POST
$n_cases = count($_POST);

if ($n_cases >= 26)
{
    echo 'Heu... on ne peut pas cocher toutes les cases ^^';
}
else if ($n_cases == 0)
{
    echo 'Merci de cocher au moins une des cases ^^';
}
else
{
    $alphabet  = str_split('abcdefghijklmnopqrstuvwxyz');
    $post_keys = array_keys($_POST);

    $res = '';

    foreach ($post_keys as $lettre)
    {
        if (in_array($lettre, $alphabet))
        {
            $res .= $lettre.' / ';
        }
        else
        {
            exit('Aie, le formulaire est corrompu !');
        }
    }

    echo 'C\'est OK ^^<br><br>';
    echo 'Vous avez choisi la (les) lettre(s) : ', $res;
}

echo '<br><br><a href="page.html">Retour</a>';
+0 -0

Cet exercice m’a donné du fil à retordre, mais je pense avoir trouvé une solution valable :

page2.php :

Afficher/Masquer le contenu masqué
<?php

$alphabet   = str_split('abcdefghijklmnopqrstuvwxyz');
$post_keys  = array_keys($_POST);
$difference = array_diff($alphabet, $post_keys);

$n_diff = count($difference);

if ($n_diff == 26)
{
    echo 'Merci de cocher au moins une des cases ^^';
}
else if ($n_diff == 0)
{
    echo 'Heu... on ne peut pas cocher toutes les cases ^^';
}
else if ($n_diff + count($post_keys) == 26)
{
    echo 'C\'est OK ^^<br><br>';
    echo 'Vous avez choisi la (les) lettre(s) : ', implode(' / ', $post_keys);
}
else
{
    exit('Aie, le formulaire est corrompu !');
}

Qu’en pense les abonnés du sujet ? ^^

Pour revenir à ton code, je n’ai pas bien compris pourquoi tu es certain à 100% que cette ligne fonctionne :

$allAreGiven = !(bool)($difference);

Convertir un tableau en boolean, ça ne donne pas des résultats inattendus ?

+0 -0

ALors tu as fait l’exercice avec suffisament de sérieux pour ne pas penser comme moi et ça j’aime.

question 1

ça marche vraiment $allAreGiven = !(bool)($difference);?

oui, il faut savoir qu’en php si tu transforme en booléen certaines valeurs ça te donne un résultat assez facile à manipuler.

Voici un ensemble d’exemple, c’est la fin de la semaine et je ne suis pas en train de faire du php donc ça ne sera pas exhaustif mais l’idée est là :

<?php
var_dump((bool)[]); // false
var_dump((bool)[0]); // true, et ça sera true pour tout tableau qui a au moins un élément
var_dump((bool)""); // false
var_dump((bool)"bla")); // true, et ça sera true pour toute chaine de caractère qui aura au moins un caractère
var_dump((bool)0); //false
var_dump((bool)1); // true, et ça sera vrai pour tout nombre qui ne vaudra pas 0
var_dump((bool)null); // false
question 2

et ma solution $n_diff + count($post_keys) == 26

eh bien, elle est bonne. EN soit, elle fait le travail.

que vaut ndiff?

le nombre de lettre qui ne sont pas dans post. si post, ne vaut que les lettre qu’on a trouvé, alors count($post_keys) vaut la quantité de lettre qu’il n’y a pas dans diff et donc le complément par rapport à ndiff bravo;

moi je te proposais de dire : $onlyAuthorized = (bool) (array_diff($post_keys, $authorized));. Si $post_keys ne contient que des clefs autorisée, la différence sera alors une liste nulle.

Je ne prétend pas que ma solution est plus élégante.

Néanmois, un conseil que je te donne pour t’aider à progresser : évite les calculs contrintuitifs dans les conditions.

Je sais, c’est facile à utiliser, et @cepus est témoin que parfois je le fais aussi quand je développe.

Mais ces calculs, tu ne les comprends que parce que tu es en train de les coder. Dans deux jours, ou même bêtement quand tu auras quitté la pièce dans laquelle tu es1, ils te paraîtront moins clairs.

Du coup, pour les clarifier ,tu as deux outils : les commentaires (pour globalement expliquer ton raisonnement), ou simplement les noms des variables (ou les deux à la fois).

Imagine, ici tu mettais : $onlyAuthorized = ($n_diff + count($post_keys) == 26);, et qu’ensuite dans ton if tu mettais if(!$onlyAuthorized){ echo "formulaire corrompu";}, quand tu aurais relu ton code, tu aurais compris ce que tu étais en train de tester. Puis si un jour tu as besoin de savoir comment tu sais que c’est $onlyAuthorized, tu remontes à l’initialisation de la variable et voilà.

  1. c’est le même effet que quand tu ne te souviens plus de ce que tu venais faire dans une pièce quand tu viens d’y rentrer

    ↩
+0 -0

oui, il faut savoir qu’en php si tu transforme en booléen certaines valeurs ça te donne un résultat assez facile à manipuler.

L’usage de cette technique est-elle considérée comme une bonne pratique ?

Du coup, ça vaudrait la peine que je me penche dessus ?

$onlyAuthorized = (bool) (array_diff($post_keys, $authorized));

Je me disais, peut-être pourrait-on écrire à la place :

$onlyAuthorized = (array_diff($post_keys, $authorized) != []);

+0 -0

oui, il faut savoir qu’en php si tu transforme en booléen certaines valeurs ça te donne un résultat assez facile à manipuler.

Personnellement, je suis assez partisan du fait que ce n’est pas parce que la sémantique d’un langage t’autorise des trucs que c’est une bonne idée de l’utiliser. Par exemple, là :

!(bool)($difference)

On est d’accord que ce que tu veux exprimer, en fait, c’est

$difference != 0

non ?

Si c’est le cas, je trouve la seconde version beaucoup moins capilotractée.

+2 -0

Pareil. (count($difference))

Et à choisir entre la solution proposée par A-312 (modulo les fautes de frappes, les fautes d’accord et le mélange français/anglais) et la solution censée être compréhensible d’un coup d’oeil mais que je dois relire 3–4x avant de comprendre :

le bon sens du premier coup d’oeil, je pense.

<?php

$lettres_autorises = ['a', 'b', 'c', 'y', 'z'];
$post_keys = array_keys($_POST);
$difference = array_diff($lettres_autorises, $post_keys);
$allAreGiven = !(bool)($difference);
$atLeastOneIsGiven = $difference != $lettres_autorises;

je préfère largement celle d’A-312. Que pour le coup je trouve compréhensible d’un coup d’oeil. Ça : $atLeastOneIsGiven = $difference != $lettres_autorises;, c’est à dire "au moins une lettre est choisie SI les lettres disponibles qui ne sont pas les lettres choisies ne sont pas les lettres disponibles", … ça marche quand tu l’écris, mais va relire ça ou débugger ça …

Pareil quand tu tombes sur !(bool)($foo). C’est un idiome en PHP ? Je me tiens pas au courant de l’actualité du langage PHP du coup quand je vois ça je me demande quelle spécialité ils ont trouvé pour avoir besoin de ce (bool). S’il est pas utile, j’éviterais de le mettre, ça évite aux lecteurs de se demander pourquoi il est là. Ou alors on y va à fond et on fait (bool)!(bool)($foo);)

+0 -0

Salut @dab,

Je pensais que les noms des champs HTML ne pouvaient pas être des nombres, mais je me suis trompé.

Du coup, suivant ta suggestion, je propose ceci :

page1.php :

Afficher/Masquer le contenu masqué
<!DOCTYPE html>
<html lang="fr">
<head>

    <meta charset="utf-8">
    <title>page1.php</title>

</head>
<body>

    <form action="page2.php" method="post">

        <?php

            $inputs = '';

            // Nombre de champs dans le formulaire
            $_MAX_INPUTS = 30;

            for ($i = 1; $i <= $_MAX_INPUTS; $i++)
            {
                $inputs .= $i.' <input type="checkbox" name="'.$i.'"><br>';
            }

            echo $inputs;

        ?>

        <input type="submit" value=" OK ">

    </form>

</body>
</html>

page2.php :

Afficher/Masquer le contenu masqué
<?php

// Nombre de champs dans le formulaire
$_MAX_INPUTS = 30;

$post_keys  =   array_keys($_POST);
$difference =   array_diff(range(1, $_MAX_INPUTS), $post_keys);
$n_diff     =   count($difference);

if ($n_diff == $_MAX_INPUTS)
{
    echo 'Merci de cocher au moins une des cases ^^';
}
else if ($n_diff == 0)
{
    echo 'Heu... on ne peut pas cocher toutes les cases ^^';
}
else if ($n_diff + count($post_keys) == $_MAX_INPUTS)
{
    echo 'C\'est OK ^^<br><br>';
    echo 'Vous avez choisi la (les) case(s) : ', implode(' / ', $post_keys);
}
else
{
    exit('Aie, le formulaire est corrompu !');
}
+0 -0
Connectez-vous pour pouvoir poster un message.
Connexion

Pas encore membre ?

Créez un compte en une minute pour profiter pleinement de toutes les fonctionnalités de Zeste de Savoir. Ici, tout est gratuit et sans publicité.
Créer un compte