ok, c'est parti. Dans le désordre:
1 - Absence de contrôle
1 2 3 4
| $query = mysql_query("SELECT id,nom FROM add_sorts_magicien ORDER BY nom");
while( $row = mysql_fetch_array($query) ){
$form->SelectOption($row['id'],$row['nom'],false);
} |
Vu qu'aucun traitement n'est prévu, si pour une raison quelconque la requête en base échoue au mieux ça lancera des erreurs, au pire ça compromettra le comportement obtenu.
2 - Indentation farfelue
Il faut faire toute une gymnastique mentale pour comprendre ce que ce script fait. Lire du code source doit être aussi aisé que lire un bouquin; le code doit être correctement indenté, aéré et commenté ce qui n'est clairement pas le cas.
3 - Utilisation des tableaux pour la mise en page
1 2 3 4 5 6 7 8 9 10 11 12 13
| <table width="650" border="0" >
<tr>
<td width="277" >Niveau <h4 id="txtniveau" >'.$sort->niveau.'</h4></td>
<td colspan="5"></td>
<td><em>'.BoolReversible($sort->reversible).'</em> </td>
</tr>
<tr>
<td><b>Ecole(s)</b> : '.$sort->ecole_text.'</td>
<td colspan="5"> </td>
<td><b>Source</b> : '.$sort->source_text.'</td>
</tr>
</table> |
L'utilisation des tableaux pour résoudre une problématique de mise en page est dépassé depuis plus de 10 ans. Il est de rigueur d'utiliser une sémantique correcte pour son code HTML.
4 - Pas de séparation entre les traitements et le rendu
Mélanger allègrement les traitements (extraction & manipulation de données) et l'affichage (production de flux HTML) est extrêmement dommageable: ça nuit à la lisibilité du code et surtout ça empêche d'avoir un contrôle sur le déroulement de la requête HTTP car il devient impossible de manipuler les en-têtes une fois le moindre contenu affiché.
5 - Inlining du CSS
1 2 3 4 5 6 7
| echo '<style>
#txtcentre label{
display:block;
font-family : \'ChantelliAntiqua\';
margin:4% 3% 1%;
}
</style>'; |
Il est déconseillé de mettre le CSS directement dans la page web ça empêche le navigateur de mettre en cache la feuille de style et donc occasionne une charge réseau supplémentaire parfaitement inutile. De surcroit, le CSS est ici inséré dans le body ce qui est incorrect vis-a-vis des standards W3C.
6 - Utilisation de balises sémantiquement inutiles
L'incorporation volontaire de balises destinées à impacter la mise en forme est à proscrire, il faut plutôt utiliser correctement les marges grâce aux règles CSS.
7 - Inclusion manuelle des classes
1 2
| include(CLASSES_PHP.'/formulaire.classe.php');
include(CLASSES_PHP.'/sortADD2.classe.php'); |
Si pour une raison quelconque la classe est déjà chargée, le script va partir en erreur fatale car on peut définir deux fois la même classe. Il serait de bon ton d'utiliser un autoloader ou au moins une directive include_once (ou plutôt require_once dans notre cas)
8 - Utilisation de include au lieu de require
Les classes sont effectivement mandataires pour le bon fonctionnement du script. Contrairement à la directive require, la directive include n'arrêtera pas le script si l'inclusion échoue, hors ces classes sont utilisées donc si elles ne sont pas correctement chargées, le script partira en erreur fatale.
9 - Absence de clarté dans la nomenclature
Quand on rédige du code, il est de bon ton d'utiliser des conventions et de s'y tenir. Ici, on retrouve des conventions différentes pour les membres de classes, du français et de l'anglais, des constantes en majuscules et en minuscules... Bref, il faudrait y mettre bon ordre.
10 - Utilisation du tag fermant ?> en fin de script
Pour un script qui n'utilise que du PHP, il est en général admis de ne pas fermer le tag PHP avec ?> ceci afin d'éviter les problèmes avec des caractères comme le saut de ligne juste derrière qui sont considérés comme du contenu (et qui provoquent l'envoi des headers HTTP).
11 - Mélange procédural / objet
1 2
| $form->BeginSelect('<h4>Choix du sort</h4>','id_sort');
$query = mysql_query("SELECT id,nom FROM add_sorts_magicien ORDER BY nom"); |
Bien qu'on utilise des classes pour la génération du formulaire, on utilise également l'extension mysql_ pour les accès bases. L'utilisation de cette extension est désormais déconseillée au profit de mysqli et dans notre cas, on devrait utiliser l'extension objet PDO.
12 - Mauvaise syntaxe pour include
La syntaxe correcte est:
include "mon_script.php"; // sans parenthèses
Pareil pour require, include_once et require_once d'ailleurs.
13 - Production de flux HTML à grand coups de concaténation
Comme dit précédemment, il est plus naturel de séparer le PHP et le HTMl et de ne pas produire le HTML à grand renforts de chaines de caractères, si possible il faut mettre le PHP et le HTML dans des fichiers à part (un pour les traitements et un pour la vue).
Conclusion:
Ce script n'est pas totalement inutile: il peut être utilisé comme mauvais exemple. Il condense en peu de lignes presque tout ce qu'il ne faut pas faire en regard des bonnes pratiques du PHP, de l'accès aux bases de données et du respect des standards du W3C. Il ne lui manque finalement qu'une faille XSS ou d'injection SQL pour compléter le tableau.
Ce genre de code véhicule l'image négative de PHP en tant que langage impropre à l'utilisation en milieu industriel et jette l’opprobre sur les développeurs consciencieux et respectueux des normes et des standards.
Sois toujours exigeant dans la qualité du code que tu produit. Le mode de fonctionnement "faisons un prototype bien sale maintenant et améliorons le plus tard" est tout simplement absurde: on ne fait pas de l'or avec du plomb.
Les normes, règles, bonnes pratiques, conventions et standards n'existent pas pour faire joli et ne sont pas destinées qu'a quelques esthètes dans leur tour d'ivoire. Elles existent dans le but de permettre l'interopérabilité, la réutilisation, l'efficacité, la lisibilité, la performance, la justesse, l'évolutilivité, la cohérence et surtout la l'intégrité du code produit. Il faut toujours coder en pensant que celui qui va maintenir ce que tu fais est un serial-killer qui connait ton adresse.
Quand tu pense économiser 1h en faisant un code sale, tu en perds en réalité 8 à le maintenir. Quand tu investis 1h de plus pour faire un code propre, tu en gagnes 7 à le maintenir. A toi de voir si tu veux passer ton temps à faire les choses proprement ou le perdre à maintenir du code sale...
Partager