CLASSE MYSQL UTILISANT LES FONCTIONS PDO
cs_fenoril
Messages postés
23
Date d'inscription
vendredi 28 mars 2008
Statut
Membre
Dernière intervention
12 juin 2011
-
17 sept. 2010 à 19:45
Vince66 Messages postés 28 Date d'inscription mardi 10 février 2004 Statut Membre Dernière intervention 5 octobre 2011 - 20 sept. 2010 à 11:01
Vince66 Messages postés 28 Date d'inscription mardi 10 février 2004 Statut Membre Dernière intervention 5 octobre 2011 - 20 sept. 2010 à 11:01
Cette discussion concerne un article du site. Pour la consulter dans son contexte d'origine, cliquez sur le lien ci-dessous.
https://codes-sources.commentcamarche.net/source/52296-classe-mysql-utilisant-les-fonctions-pdo
https://codes-sources.commentcamarche.net/source/52296-classe-mysql-utilisant-les-fonctions-pdo
20 sept. 2010 à 11:01
Tout d'abord merci d'avoir pris le temps d'étudier mon code et de rédiger une critique très constructive :)
Pour te répondre Neigedhiver sur la gestion des erreurs, j'ai commencé à développer la classe pour des besoins perso et ça m'allait bien d'avoir un retour immédiat en cas d'erreur. Mais je me rends compte effectivement qu'à partir du moment où je la partage, il est plus logique de séparer la gestion des erreurs et la gestion des données pour qu'elle soit modulaire. Je vais travailler dessus afin d'aller dans ce sens.
Pourquoi ne pas avoir étendu PDO ? C'est surement très bête mais tout simplement parce que je n'y avais pas pensé. C'est aussi ce que je recherchais en déposant ma source sur CodesSources : avoir des retours constructifs qui me permettent de l'améliorer et corriger des points auxquels je n'avais pas pensé :)
Donc merci.
Pour la méthode Query2Array, le but était de simplifier le format de sortie des données afin de parcourir celles-ci avec un simple for ou foreach. En revanche, je comprends tout à fait la problématique des performances dégradées dues au double parcours des données.
Holala tu as entièrement raison pour le bout de code que tu cites, je n'avais absolument pas pensé au implode. Mea Culpa !
Merci pour la méthode InTable, en effet c'est plus simple.
Enfin pour vous répondre à tous les deux, dans la décomposition de requête, l'idée était encore une fois de simplifier la construction des diverses requêtes. Cependant, j'admets que cela manque de souplesse lorsque l'on veut faire des requêtes plus complexes. C'était une expérimentation très maladroite, j'avoue.
Rassure toi, j'aurais du mal à prendre pour "destructive" une critique aussi détaillée et argumentée ;)
Une remarque du style "C'est nul ! Retourne faire du Word", je l'aurais moins bien pris c'est sûr :D
Je vais prendre en compte vos remarques et essayer de faire une classe plus cohérente.
Merci encore !
18 sept. 2010 à 01:37
Pour reprendre ce que dis Fenoril, ta décomposition de requêtes m'étonne : c'est comme si tu mélangeais ORM et DAO dans une sourcouche de PDO... Un joyeux mélange pas forcément facile à manipuler avec cette classe, en l'état...
18 sept. 2010 à 01:33
Le code est propre, documenté, tu as choisi d'utiliser PDO : que des bons points.
Cependant, je note, vite-fait, des erreurs de conception :
1/ Dans le constructeur, tu mets un bloc try...catch : même si ça fonctionne (heureusement !) ce n'est pas très correct, dans la conception : les exceptions n'ont pas de raison, fondamentalement, d'être traitées dans une classe comme celle-ci. Ta classe est une surcouche de PDO (on y reviendra plus tard) ; PDO lève des exceptions => pourquoi ta classe les intercepterait-elle ? La seule raison valable serait : les traiter de manière plus approfondie (log dans un fichier, utilisation d'un cache, etc). Or, on ne peut pas dire qu'un vulgaire echo (pardon pour lui) soit plus approfondi qu'une véritable gestion d'exception. Conclusion : tu peux tout à fait attraper une PDOException, si tu lèves une exception perso DbException (ce qui resterait cohérent). Donc au lieu de faire un echo, lève une exception à toi : c'est à l'utilisateur de décider comment il les gère, pas à toi de lui imposer l'affichage d'un texte alors que si ça se trouve, i la décidé de ne rien faire afficher du tout par son script (genre je traite les données d'un formulaire, je n'affiche rien, puis je redirige avec header()... sauf que paf, tu as forcé l'envoi des entêtes HTTP, le script plante complètement).
2/ Comme je le disais, ta classe est une curcouche de PDO. Pourquoi ? Pourquoi ne pas simplement étendre PDO ? Tu ne fais même pas un véritable wrapper, puisque tu limites les méthodes disponibles... Pour faire un véritable wrapper, il faudrait au minimum utiliser la méthode magique __call() pour pouvoir utiliser les méthodes de PDO. Ta classe ne permet pas de se passer des méthodes de PDO que tu ne remplaces pas...
3/ Ta méthode Query2Array() me paraît inutile en plus d'être mal codée. De même que pour le constructeur, tu devrais lever une nouvelle exception plutôt que simplement intercepter celle qui se présente et afficher un message d'erreur. Qui plus est, si une erreur SQL se produit, tu lèves une exception standard : ce n'est pas rendre service à l'utilisateur qui ne va plus savoir quand intercepter des exceptions ni lesquelles peuvent être attrapées.
Par ailleurs, PDO permet d'itérer sur les résultats PDOStatement, ce que ne permettait pas l'extension mysql sans une classe supplémentaire : pourquoi tout récupérer dans un tableau ? C'est non seulement superflu, mais surtout non performant : PDO va itérer une première fois sur le résultat pour remplir le tableau, puis l'utilisateur va itérer sur le tableau pour traiter les résultats (et il est capable d'itérer encore pour les afficher... sisi, ça arrive, j'en ai vus).
Bon, d'autres détails qui me gênent un peu...
Dans ta méthode makeSQLPart() :
# foreach ($Values as $Value)
# {
# //On ajoute la valeur courante du tableau
# $Sql .= $Value;
# //Si nous ne sommes pas à la fin du tableau
# if (($Key) != count($Values))
# //On ajoute le séparateur
# $Sql .= $separator;
# //On incrémente de un pour l'élément suivant
# $Key++;
# }
Pouah, c'est répugnant... dire qu'on peut faire ça en une ligne (de manière certainement plus optimisée en terme de performances) en une seule ligne avec implode() (et en plus c'est bien plus lisible !).
Dans la méthode InTable() :
return count($result) ? true : false;
Je te propose :
return (bool) $this->GetFrom($Table, $Field, false, $Field.'` LIKE "'.($Strict?$Needle:'%'.$Needle.'%').'"');
D'une manière générale, essaie de proscrire la gestion d'erreur dans la classe avec des echo : une classe permet à l'utilisateur de modulariser son code ; il ne FAUT PAS l'obliger à afficher quelque chose s'il ne le souhaite pas.
Pour ta méthode GetError(), tu devrais simplement retourner le message d'erreur, avec le minimum de mise en forme. Ou alors, laisse l'utilisateur modifier le masque, via une propriété avec un setter, plutôt que de lui imposer un formatage html avec et
: si je veux afficher ça dans une liste
? Ou dans un
avec un id spécifique, pour le mettre en forme avec un CSS ?
Bon... Ne vois pas dans mon commentaire une critique destructrice de ta source : l'intention est louable, c'est plutôt bien codé et surtout très clair (ce qui est rare quand même !!). Vois-y plutôt un peu de matière pour pouvoir améliorer ta source et la rendre plus intéressante (y compris pour toi). Je te donne des conseils qui doivent également te permettre de coder de manière plus souple et modularisable, en séparant autant que possible l'affichage des données de leur traitement. Ta classe ne sert pas à afficher, juste à traiter : laisse l'utilisateur afficher comme il l'entend.
Et n'hésite pas à utiliser des classes d'exceptions personnalisées qui pourront réellement servir à l'utilisateur (parce que c'est à lui qu'il faut penser, pas au développeur).
Bonne continuation !
17 sept. 2010 à 19:45
N'est-ce pas un peu dommage de ne pas utiliser les requètes préparées afin d'obtenir une sécurisations satisfaisante en plus de l'automatisation ? Y aurait-il une contrindication que je n'aurais pas compris ? (Ce n'est aucunement de l'ironie, je suis pas assez fort pour voir tous les détails...)
Ceci dit, je ne vois pas l'intéret de la décomposition des requètes sql... Qu'y trouves-tu ?
Par ailleurs merci pour le partage de code.