20 mauvaises pratiques de développement quand on développe avec Ruby on Rails

Ruby on RailsLe 30 mars, Chad Fowler a demandé sur Twitter Rails programmers: what’s an example of one thing you find in other people’s Rails code that you (almost) always consider to be wrong?. Les réponses ont fusé, donnant une liste de 20 mauvaises pratiques à destination des développeurs Ruby on Rails, mais également de tous ceux qui développent avec un framework MVC.

Quelles sont ces 20 mauvaises pratiques ?

  1. Le code dans les vues (je plussois).
  2. L’indentation avec des tabulations au lieu d’espaces, et les erreurs d’indentation.
  3. Les tests absents, ou non pertinents.
  4. La mauvaise utilisation des associations polymorphiques et de l’héritage simple.
  5. La gestion d’exception à l’arrache.
  6. Les assignations accidentelles de variables locales.
  7. Le code qui entraîne des avertissements de la part de Ruby.
  8. Le code qui utilise les fonctionnalités de Rails :-)
  9. La mauvaise utilisation de validates_uniqueness_of.
  10. Le code mis au mauvais endroit (du HTML dans le contrôleur, les requêtes SQL dans les helpers…)
  11. Les fichiers de configuration contenant leur propre définition d’environnement.
  12. Trop de code, surtout pour faire quelque chose de simple.
  13. Bidouiller $LOAD_PATH.
  14. La logique obfusquée.
  15. Insérer des données dans les migrations (là, je ne suis pas d’accord).
  16. L’envoi de données dans des requêtes GET. Ce point est cependant à nuancer.
  17. Ne pas utiliser de transactions.
  18. La mauvaise utilisation du Javascript, comme le Javascript bloquant.
  19. Les méthodes trop longues.
  20. Les commentaires trop verbeux.

Je rejoins la plupart des éléments énoncés dans ce sondage, et cela fait partie des travers dans lesquels j’essaie de ne pas tomber. Et vous, qu’en pensez-vous ? Tombez-vous souvent dans un de ces pièges ? Voyez-vous d’autres mauvaises pratiques qui pourraient rejoindre cette liste ?

  • Gravatar

    Par burningHat 07/04/2009 at 07h41

    Hi,

    C’est peut-être tout bête mais le point 8 n’est pas très clair pour moi… Qu’est-ce qu’on entend par “le code qui utilise les fonctionnalités de Rails” et pourquoi ne faut-il pas s’en servir ?!?


  • Gravatar

    Par Loïc Chollier 07/04/2009 at 09h01

    Je pense qu’il veut dire qu’il ne sert à rien de ré-inventer la roue :)

    Moi c’est le point numéro 2, pourquoi considère-t-on cela comme une mauvaise pratique ?


  • Gravatar

    Par NiKo 07/04/2009 at 09h19

    C’est marrant, tu prends les mêmes et tu remplaces ruby par php et rails par symfony - lorsque’applicable et à l’exception notable du point #8 (qui m’hallucine un peu) - et t’as un billet tout aussi pertinent :-)


  • Gravatar

    Par Emilien 07/04/2009 at 10h33

    Salut, Idem, je ne suis pas trop d’accord avec le 15 (données dans les migrations). On peut par exemple vouloir synchroniser l’insertion de nouvelles rubriques avec la modif du css que ça implique.

    Pas compris le 14, “logique obfusquée” par contre.


  • Gravatar

    Par Emilien 07/04/2009 at 10h38

    Arf, on dirait que mon commentaire s’est perdu dans la nature, je recommence :

    Idem, je ne suis pas trop d’accord avec le 15 (données dans les migrations). On peut par exemple vouloir synchroniser l’insertion de nouvelles rubriques avec la modif du css que ça implique. A consommer avec modération bien sûr.


  • Gravatar

    Par Sly 07/04/2009 at 10h40

    Bonjour à tous,

    j’ai lu l’article de Chad Fowler hier et je dois avouer que je ne suis pas d’accord avec tous les points, ceux qu’il mentionne lui-même comme étant les plus critiques d’ailleurs.

    1) Le code dans les vues (je plussois). Au moins, celui-ci met tout le monde d’accord ;)

    6) Les assignations accidentelles de variables locales. Ça arrive encore ça ? Non, sérieusement…

    15) Insérer des données dans les migrations (là, je ne suis pas d’accord). Complètement. Pour certains cas ce serait même limite une Best Practice. Exemple : une table avec les civilités (Mlle, Mme, M, Dr, Prof, SAS…)

    8) Le code qui utilise les fonctionnalités de Rails :-) Celui-ci est intéressant. Je vote plutôt pour l’inverse : une fonctionnalité de Rails vient avec un certain lot de “promesses” : elle est reconnue, testée et sera certainement maintenue dans le futur. Pourquoi alors réinventer la roue ?

    Nous sommes nombreux à vouloir limiter les dépendances, je pense notamment au mouvement qui voudrait voir enfin une intégration complète de la pagination dans le framework.

    Certes, nous sommes aussi sujets à des surprises et changements d’avis, parfois de DHH seul, comme le choix de “try” au lieu de “andand” ou de la méthode “tortoises all the way down” que j’avais trouvées élégantes et astucieuses. Mais on survit, et pour plusieurs de ces changements (le passage de 1.2.6 à 2.0 et l’internationalisation par exemple) on peut s’en sortir avec un gros script de remplacement automatique.

    Le seul vrai souci que j’ai eu c’est la promesse du Nested Model Assignment en Rails 2.1, abandonnée (silencieusement, ça se contentait de rater lamentablement) en 2.2 et retrouvée pour Rails 2.3 : en attendant nous avons développé notre propre version, certes robuste mais qui n’avait aucune garantie de ne pas créer de conflit avec les évolutions futures de Rails… Lors du passage à 2.2, tout le monde s’est perdu en diverses approches dont aucune ne fonctionnait vraiment, genre déclarer dans le modèle que tel autre était “nestable” et devoir activer/désactiver les validations du père ou de l’enfant… selon le résultat que vous vouliez atteindre.


  • Gravatar

    Par Zaphod Beeblebrox 07/04/2009 at 12h25

    Idem, comprends pas le point 8 …


  • Gravatar

    Par Frédéric de Villamil 07/04/2009 at 13h19

    @burninghat, @niko, @sly, @zaphod : pour le point 8, je ne citerai que deux exemples, curieusement ceux qui ont fait la notoriété de Rails au tout début : scaffold et linktoremote.

    Scaffold est génial pour générer des formulaires vite fait qui seront utilités afin de peupler les bases, mais honnêtement inutilisable dans le monde réel. D’une part, on suit une logique CRUD pure au lieu de la logique métier dont on pourrait avoir besoin. Et surtout, on génère du code que l’on passera plus de temps à modifier pour le rendre utilisable en production que si on était partis de zéro.

    Linktoremote est génial pour faire des appels AJAX simples, mais dès que l’on veut insérer des traitements, ou complexifier les choses, c’est la mission. Et je ne parle pas des problèmes de compatibilité rencontrés quand on souhaite passer de Prototype à autre chose… Ce sont même deux des raisons qui ont fait que linktoremote, et d’une manière tous les helpers générant du JS n’ont pas été repris dans (cette coquille vide de) Merb.

    @sly, @Emilien Concernant les données dans les migrations, j’aurais tendance à être (presque) d’accord. C’est sale, et il faudrait faire autrement, par exemple en utilisant une tâche rake dédiée. Mais c’est super pratique, et ça permet d’être certain de ne jamais insérer les données de base deux fois.

    @Loïc : identer avec des tabs au lieu d’espaces va te bousiller ton indentation pour peu que le code soit édité dans un autre éditeur. Avec les espaces, pas de danger.


  • Gravatar

    Par sly 07/04/2009 at 15h15

    En même temps, le scaffold dynamique a très vite été identifié comme LE truc à ne pas faire.


  • Gravatar

    Par Fabien 16/04/2009 at 10h45

    Bonjour,

    c’est bizarre ces polémiques sur l’utilisation de l’espace de la tabulation et le placement des acollades… Personnellement j’utilise les tab pour une raison : On peut choisir la taille de l’indentation contrairement aux espaces. Alors pourquoi rester sur du vieux parce que Emacs gère mieux les espaces?


  • Gravatar

    Par Philippe 27/04/2009 at 15h21

    @Fabien

    Certains cherchent à limiter leur code à 80 caractères. Impossible de gérer cela avec tabs allant de 2 à 8 caractères.

    Il arrive que l’on split une longue ligne de code sur plusieurs. Par exemple: @message = mock_model(Message,

                      :save => true, 
                      :name => "MyProject!")
    

    De même, des tabs allant de 2 à 8 caractères empêche de rendre cela. (Et avec une police de type monospace ça passerait mieux dans ce commentaire! :) )

    Enfin, les editeurs de texte permettent de naviguer dans les espaces comme des tabulations. (que ce soir Emacs, Vim ou Netbeans)


Commentaire 20 mauvaises pratiques de développement quand on développe avec Ruby on Rails