fév
01

Quizz PHP : Correction

Adrien Mogenet Php 2010-02-01

correction

Vous avez récemment découvert mon histoire d’un curieux recrutement. J’y montrais alors une portion de code PHP exemptée de bonne pratique et fourmillant d’erreurs. This might sound more complicated than playing online casino (http://www.casino.com/fr/) at first but you will soon get your head around it. Voici donc la « correction » que je vous propose, recensant les points que j’aurais aimé voir soulevés par les candidats :

  1. Le tag <?php dans sa version courte
  2. Un code intégralement en français.
  3. Pas de commentaires multi-lignes en entête de fonction
  4. Pas de tags pour la génération de documentation (phpDoc, Doxygen…)
  5. Pas de typage pour le paramètre $voiture (en tant qu’objet, autant bénéficier du typage apporté par PHP)
  6. Commentaires inutiles dans le code (lignes 8 et 13)
  7. Mauvaise indentation
  8. Problème de type pour $max
  9. If/else à problème
  10. L’opérateur de comparaison pourrait être poussé à « ===« 
  11. Lorsque que $max vaut « inconnu », on pourrait préférer une constante
  12. Pas d’utilisation d’ORM, utilisation de mysql_query natif
  13. Requête SQL peu lisible (mots clés en minuscules, une seule ligne…)
  14. Problèmes dans la requête SQL (guillemets mal placés, le SET est après le WHERE)
  15. Pas de protection de la requête
  16. return est un mot clé, pas une fonction : on devrait omettre les parenthèses.
  17. Il vaut mieux éviter d’insérer les tags de fermeture PHP
  18. Vu ce que fait la fonction, ça serait sûrement mieux d’en faire une méthode au sein de la classe (mais l’analyse de ce que fait la fonction n’était pas demandé)

Bravo à tous ceux qui avaient farfouillé ! Sans forcément trouver exactement tous ces points, il était sûrement aisé d’en identifier au moins 4 ou 5. Attention, ce ne sont pas forcément des erreurs, mais des points qui pourraient être discutés (les commentaires pourraient très bien être en français pour telle ou telle raison, par exemple)

Address: http://www.ze-technology.com/2010/02/01/quizz-php-corrections/
«
»
Trackback

7 commentaires jusqu'a maintenant

  1. Alexandre Salomé @ 2010-02-02 09:25

    4 ou 5… t’es gentil :)

  2. Adrien Mogenet @ 2010-02-02 10:28

    Tu me rassures !!

  3. JP @ 2010-02-03 08:20

    Il faut que ca reste entre nous mais moi j’en avais trouve 7…(non serieux j’y comprends rien mais j’ai bien rigole en lisant l’entretien d’embauche)

  4. tight @ 2010-02-09 21:30

    C’est une excellente idée ce code (il est vraiment horrible ;) )

    Je pense qu’il y a d’autres points discutables : le nom de la fonction et son commentaire (on ne parle de BDD nulle part), la requête n’a rien à faire là (et son retour n’est pas testé).
    Aussi, pour ce genre de test, je préfère passer par un tableau (une propriété en protected dans la classe) (par contre, je développe en frenglish :P )

  5. Amaury @ 2010-03-23 11:38

    2 => Il y a des entreprises qui encouragent à un code en français. Personnellement, je n’aime pas. Mais on peux difficilement élever ça à une « mauvaise pratique ».

    3,4 => Ah oui, le nombre de jeunes programmeurs qui ne connaissent ni Doxygen ni PHPDoc !…

    6 => Mouais, sujet à discussion. Habituellement, on considère que la documentation au-dessus de la fonction doit expliquer ce que fait cette fonction (et non comment elle le fait). Par contre, expliquer à l’intérieur comment l’algorithme a été pensé, c’est une bonne chose. Je suis d’accord que dans ton exemple les commentaires sont inutiles ; mais là encore, c’est difficile de dire que les commentaires dans le code sont une mauvaise pratique.

    11 => Ou bien on pourrait retourner NULL. Ou lever une exception…

    12 => Un ORM mal utilisé (surtout par de jeunes développeurs) est bien pire que l’utilisation demysql_query directe. Bon, le minimum serait de passer par une librairie d’abstraction. Mais dans le cadre de ton test, avec juste un bout de code sorti de tout contexte, c’est difficile de leur en vouloir.

    16 => L’argument n’est pas valable. « if » est un mot-clé du langage, et il a besoin de parenthèses. Là on est dans le domaine de la norme de codage, ça revient à discuter des goûts et des couleurs.

    17 => Argument très discutable. C’est sûr que pour les développeurs étourdis, ça évite d’avoir des espaces qui s’ajoutent sur la sortie standard. Mais ça reste globalement malpropre, et pourrait poser des problèmes avec certains compilateurs PHP.

    18 => Vu les problèmes de logique (intentionnels, je sais) de cette fonction, elle mériterait surtout d’être effacée ! ;-)

  6. Adrien M. @ 2010-03-24 22:49

    Salut Amaury !

    D’une manière générale, je ne parlais pas forcément de mauvaises pratiques, mais juste de « points discutables », qui méritent d’être formalisés dans une entreprise, si ce n’est au niveau d’une communauté entière.

    Pour le return, cependant, je suis moins mitigé que toi ! Je me base sur les dires du manuel : http://php.net/manual/fr/function.return.php

    À bientôt !

  7. Emily @ 2010-06-02 05:30

    C’est une excellente idée ce code (il est vraiment horrible ;) )

    Je pense qu’il y a d’autres points discutables : le nom de la fonction et son commentaire (on ne parle de BDD nulle part), la requête n’a rien à faire là (et son retour n’est pas testé).
    Aussi, pour ce genre de test, je préfère passer par un tableau (une propriété en protected dans la classe) (par contre, je développe en frenglish :P )

Ajouter votre commentaire