Une requête dans une boucle, est-ce "propre" ?

Par Etienne69, il y a 11 ans

PHP

Bonjour à tous,
J'ai fais un bout de code qui fonctionne bien pour une application Web perso, mais j'ai la nette impression que ma façon de coder n'est pas propre (code spaghetti), ou qu'en PDO, il y a moyen de faire plus simple...

Le but :
Avant de supprimer un aliment de la table menu_aliments je vérifie que cet aliment n'est pas utilisé dans une recette. Pour cela, je consulte la table menu_ingrédients à la recherche de l'aliment_id en question. Si je trouve un enregisrtement correspondant, j'en profite pour récupérer dans ce même enregistrement le recette_id pour enfin aller chercher dans la table menu_recettes le recette_id et le recette_nom...
J'obtiens ainsi un lien vers la fiche de recette qui contient l'aliement ne pouvant être supprimé !

Le code :

<?php // Connexion à la base de données : require("connexion.php"); // Récupération de l'id d'aliment : $id = $_GET["id"]; $aliment_nom = $_GET['nom']; // Vérification si l'aliment est utilisé dans une recette : $requete1 = $bdd->prepare("SELECT COUNT(*) as nb FROM menu_ingredients WHERE ingredient_idaliment = $id"); $requete1->execute(array('id' => $id)) or die(print_r($requete1->errorInfo())); $reponse1 = $requete1->fetch(); if (isset($reponse1['nb']) && $reponse1['nb'] == 0){ // Requête de suppression de l'aliment : $requete2 = $bdd->prepare('DELETE FROM menu_aliments WHERE aliment_id =:id'); $requete2->execute(array('id'=>$id)) or die(print_r($requete2->errorInfo())); // Message de modification d'enregistrement echo 'L\'aliment ' . $aliment_nom . ' a bien été supprimé... ' . '<br />'; echo 'Vous allez être redirigé sur la page précédente...'; $requete2->closeCursor(); } else { // Suppression impossible, Aliment utilisé dans une ou plusieurs recettes : echo 'Suppression impossible, l\'aliment ' . '<strong>' . $aliment_nom . '</strong>' . ' est utilisé dans ' . $reponse1['nb'] . ' recette(s) !'. '<br />'; $requete3 = $bdd->prepare('SELECT ingredient_recette_id FROM menu_ingredients WHERE ingredient_idaliment = :id'); $requete3->execute(array('id'=>$id)) or die(print_r($requete3->errorInfo())); while($reponse3 = $requete3->fetch()) { $requete4 = $bdd->prepare('SELECT recette_id, recette_nom FROM menu_recettes WHERE recette_id = :valeur'); requete4->execute(array('valeur'=>$reponse3['ingredient_recette_id'])) or die(print_r($requete4->errorInfo())); $reponse4 = $requete4->fetch(); echo '<a href="recette_modif2.php?rid=' . $reponse4['recette_id'] . '">' . $reponse4['recette_nom'] . '</a><br />'; } $requete3->closeCursor(); $requete4->closeCursor(); echo 'Vous allez être redirigé à la page précédente...' .'<br />'; } $requete1->closeCursor();?>

Merci d'avance pour votre aide ou conseils !

6 réponses

Huggy, il y a 11 ans

Bonjour Etienne,
Je vois 2 choses à optimiser, elles portent sur les requêtes prepare.
Les requêtes prepare sont utiles pour optimiser les requêtes qui se répètent plusieurs fois avec des paramètres différents.
Ici c'est tout à fait adapté pour ta requete4 mais tu perds toute l'optimisation si tu mets le prépare à l'intérieur de ta boucle !!!

// on prepare qu'une seule fois $requete4 = $bdd->prepare('SELECT recette_id, recette_nom FROM menu_recettes WHERE recette_id = :valeur'); while($reponse3 = $requete3->fetch()) { requete4->execute(array('valeur'=>$reponse3['ingredient_recette_id'])) or die(print_r($requete4->errorInfo())); $reponse4 = $requete4->fetch(); echo '<a href="recette_modif2.php?rid=' . $reponse4['recette_id'] . '">' . $reponse4['recette_nom'] . '</a><br />'; }

La mode est d'utiliser la commande prepare pour nettoyer les paramètres, certes, mais si ton parametre est un entier, le risque est faible, pour ta première requête , le 'SELECT COUNT ... tu peux donc faire un query direct à la place d'un prepare.
Dailleurs il y a un bug, tu as écrit directement le contenu de la variable $id et non pas un placeholder :id

mxmaxime, il y a 11 ans

Salut, effectuer des requêtes SQL dans une boucle n'est pas à faire.
Imagine le nombre de requêtes que cela peut te générer, lors du développement il faut chercher à réduire les req sql.

Huggy, il y a 11 ans

@Emix c'est pourtant ce qu'on est obligé de faire dés qu'on doit générer des json/xml à plusieurs niveaux.

Huggy, il y a 11 ans

Tu peux remplacer les requete3 et requete4 par une seule qui comporte une jointure

$requete5 = $bdd->query("SELECT R.recette_id, R.recette_nom FROM menu_recettes R INNER JOIN menu_ingredients I ON (R.recette_id = I.ingredient_recette_id) WHERE I.ingredient_idaliment = $id"); while($reponse5 = $requete5->fetch()) { echo '<a href="recette_modif2.php?rid=' . $reponse5['recette_id'] . '">' . $reponse5['recette_nom'] . '</a><br />'; } $requete5->closeCursor();
Etienne69, il y a 11 ans

@Huggy : Merci beaucoup pour tes infos...
il y a un bug, tu as écrit directement le contenu de la variable $id et non pas un placeholder :id
C'est justement parce que je venais de faire mumuse avec un query que j'ai oublié de remettre mon :id ! Merci de m'économiser
une recherche d'erreur ! Mon site étant sur un nas en local , je ne risque pas d'injection sql mais je m'efforce de programmer comme c'est écrit dans mes bouquins. D'ailleurs je n'ai toujours pas compris pourquoi le fait de préparer sa requête protège d'une injection.
Tu dis :

La mode est d'utiliser la commande prepare pour nettoyer les paramètres, certes, mais si ton paramètre est un entier, le risque est faible :
Avant j'utilisais des

$aliment_calorie = mysql_real_escape_string(htmlspecialchars($_POST['calorie']));

mais c'est inutile maintenant, si j'ai bien compris. Par contre qu'est ce qui empêche une utilisation mauvaise dans ma requête COUNT
si j'utilise directement un query et un $id ? Le paramètre attendu est bien un entier et le champs de la table un INT(5) mais comment cette manière de coder peut être dangereuse ou pas selon que l'on prépare ou non sa requête...Cette partie la reste assez obscure pour moi.

Tu peux remplacer les requete3 et requete4 par une seule qui comporte une jointure
Merci beaucoup pour cette simplification... Je vais me plonger dans mon bouquin qui en parle...mais je dois avouer que je n'ai lu que les chapitres qui m'intéressent pour ce que je veux faire (c'est pas bien je sais), c'est pour cette raison que sans ce chapitre j'étais arrivé au bout de ce que je savais faire, d'où mon code lourdingue !

@Emix

Salut, effectuer des requêtes SQL dans une boucle n'est pas à faire.
Merci à toi aussi pour ton aide...je m'en doutais bien, d'où mon post ici ;-)
il faut chercher à réduire les req sql.
Je m'y emploie, d'autant que tout passer en PDO est nouveau pour moi.

mxmaxime, il y a 11 ans

@Huggy dans certains cas très rares peut-être, mais aucun développeur va te dire "fait des requêtes dans une boucle", il faut vraiment éviter..