Ne mettez pas n’importe quoi en Open Source

18 sept 2009

Le service de raccourcissement d’url tr.im vient de rendre public le code de son application.
Je ne critique pas l’idée qui est particulièrement bonne. C’est cool

Mais pas dans la manière dont la chose est développée et qui serait presque un cas d’école.
Si on se rends à la base du projet, on vois trois applications Rails :

trim
trim_api
trim_redirect

Et en comparant trim et trim_api, on se rends compte que les deux applications sont identiques, à l’exception près que l’une retourne de l’HTML et l’autre du XML.
Urk donc. Une répétition de code particulièrement flagrante et strictement inutile.
Si, avec Rails, vous désirez retourner deux pages différentes pour le même code, une en HTML et l’autre en XML (ou tout autre format), il suffit d’utiliser respond_to.
Nous avons donc déjà une application que l’on peut fusionner avec la principale.

Quant à la troisième, qui gère les redirections, elle est nettement fusionnable à l’application principale. Il s’agit simplement d’une route par défaut.
Un petit :

map.connect ':redirect_id', :controller => 'redirect', :action => 'index'

Avec le code déjà présent dans le contrôleur redirect pour que cela fonctionne très bien.
Cette troisième application est donc également inutile. Nous avons trois applications différentes en une, et deux d’entre elles sont inutiles. Pas cool.

Enfin regardons trois dossiers similaires : les tests.
Le premier (qui est censé être le plus important vu que c’est le site) n’a strictement aucun test.
L’API a quelques tests. Je n’ai pas passé de rcov dessus cependant. Et vu leur nombre, je doute que l’on ait un pourcentage de code testé convenable.
Quand aux redirections, c’est pas testé non plus.

Alors oui, mettre ses applications en open source c’est cool.
Mais le minimum est d’éviter ce genre d’erreur de débutant (je parle surtout de la première. Le manque de tests, vu le peu de développeurs qui en font encore aujourd’hui, c’est pas spécialement étonnant) est un strict minimum.
D’ailleurs j’ai également ouvert une issue sur le projet.
Et vous vous en pensez quoi ?

Tags: audit, Développement, github, rails, ruby, ruby on rails, rubyonrails, tr.im

Cet article est rédigé par Damien MATHIEU.
Et est disponible sous licence creative common by-nc-nd.
Si vous appréciez son contenu, n'hésitez pas à me recommander.

12 Responses to Ne mettez pas n’importe quoi en Open Source

Pierre Bourdon

septembre 18th, 2009 at 15:41

Je trouve qu’au contraire, mettre son application sous une licence open source ou libre permet d’avoir des code reviews de gens qui connaissement parfois mieux le framework/le langage utilisé et qui peuvent avoir de bonnes idées.

Là, par exemple, si tr.im n’avait pas ouvert ses sources, les développeurs du projet auraient peut-être continué à dupliquer leur code inutilement. Le fait que tu ai vu que c’était inutile et que tu l’ai signalé permet d’améliorer d’une, la qualité du code, et de deux, les compétences des développeurs.

Bref, je ne vois pas en quoi il faudrait un seuil minimum de qualité pour pouvoir ouvrir son code source, alors que c’est parfois justement un bon moyen d’améliorer cette qualité.

Ads

Natim

septembre 18th, 2009 at 15:48

Je suis tout à fait d’accord avec Pierre Bourdon.
Il me parait très constructif d’avoir ce genre de retour.
Tu viens de simplifier considérablement le code de cette application, et donc les coûts de développement futurs (s’ils devaient à chaque fois modifier les deux pour corriger les deux bugs).
C’est aussi à cela que sert l’OpenSource. Nul n’est parfait.

Damien

septembre 18th, 2009 at 15:50

Tout à fait d’accord avec toi. Mais je pense qu’il faut tout de même avoir un minimum de qualité avant de rendre son application publique.
Ici le respond_to est expliqué dans le tutoriel « Getting Started » de rails. Tout développeur qui a commencé par la, c’est à dire à cherché à rendre son application plus propre a normalement lu cet article.

Alors ok pour avoir une application pas forcément parfaite. Aucune ne l’est de toute façon.
Mais je pense que dans des cas trop extrêmes (comme celui-ci), cela ne pousse justement pas les gens à participer au projet.
Au contraire, cela en donne une mauvaise image auprès des développeurs. Pour un service utilisé uniquement par des geeks, c’est particulièrement dommage.

Pierre Bourdon

septembre 18th, 2009 at 16:45

Au contraire, ça t’a poussé à participer indirectement au projet : tu as fait la remarque aux développeurs sous la forme d’un bug report .

Et puis cette mauvaise image n’est que temporaire : on peut imaginer que dans deux ou trois jours le problème sera entièrement réglé, cela grâce uniquement à l’ouverture du code. Après, tout dépend de ce que tu préfères : du code moche non améliorable ou du code moche améliorable par qui le souhaite . En tant que développeur, je préfère de loin le deuxième cas.

Damien

septembre 18th, 2009 at 22:14

Bon bah du coup de toute façon la problématique est complètement différente puisque l’auteur se contrebalance d’avoir du code propre se fournit des pseudo excuses.
http://github.com/ejw/tr.im/issues/closed/#issue/7

Palleas

septembre 19th, 2009 at 15:16

En même temps, je ne crois pas que j’aurais commencé mon ticket par « Urk », t’avais sans doute moyen d’y aller plus diplomatiquement ^^’

Jean

septembre 22nd, 2009 at 14:46

Bonjour Damien, c’est ma première intervention sur ton blog mais ce n’est pas la première fois qu’un de tes billets m’interpelle.

J’y perçoit régulièrement de la prétention sachant que tes codes ou solutions ne sont certainement pas exempts de défauts.

Je pourrais te retourner l’argument « Ne mettez pas n’importe quoi en ligne ». Par exemple, si de ton point de vue tr.im n’aurait pas dû mettre son code open source, alors tu n’aurais jamais du mettre refStats en ligne qui est une catastrophe du point de vue design (~ subjectif, ok) et comporte foultitude de bugs point de vue dév.

Il existe de grosses lacunes sur ton application, surprenantes voire risibles pour quelqu’un prônant le DRY, les test unitaires, et autres termes cools qui en jettent…

Démo : va sur refSTats, ne remplis pas les champs de login et envoie le formulaire. Conclusion ? Ré-envoie le formulaire toujours sans rien remplir. Conclusion ? Sur quelque chose d’aussi basique que ça, cela démontre que le professionnalisme que tu essaies de mettre en avant sur ton blog est à des années lumières de la réalité (en l’occurence celle que je perçois). En tous cas, ça ne prend pas avec moi, qui suis loin d’être un cador du code et qui n’en ai pas la prétention.

De plus, je ne vois pas l’intérêt que tu as de poster des articles sur la religion dans un blog (certes le tien) qui parle de code.

Voilà, réponse souhaitée merci !

« Let’s work on getting better code »

Damien

septembre 22nd, 2009 at 14:49

Concernant RefStats je suis parfaitement conscient de son absence totale d’ergonomie. C’est une chose à laquelle je travaille.

Concernant les articles « religion » que je sache, je ne suis pas le seul à avoir un blog technique et ne pas faire que des articles dessus.
D’ailleurs l’article le plus visité sur ce blog n’est absolument pas technique puisque c’est un article photo.

Jean

septembre 22nd, 2009 at 15:18

Je n’aurais pas réduit les lacunes de refStats à un simple manque d’ergonomie, je parlais de code là ! Et vu comment l’application réagit (bug trouvé les 10 premières secondes de prise en main), je doute de la propreté du code, c’est tout. « L’intégrité engendre la crédibilité. » Wayne Cheng

Damien

septembre 22nd, 2009 at 15:19

Le code de refstats (qui utilise l’API) est disponible ici
http://github.com/dmathieu/refsite-php

Mais je ne parlai pas non plus que d’ergonomie.

Pierre Bourdon

septembre 23rd, 2009 at 18:18

Les développeurs de projets libres sont des humains : si tu fais un bug report en leur disant « hey les gens, votre code est horrible, il me donne envie de gerber », ça le fait pas trop. Même si tu donnes des solutions ensuite, c’est dur de faire abstraction des « insultes-critiques » faites sur le code.

Avoir du bon code c’est important pour faire du libre, mais savoir communiquer correctement et courtoisement avec les développeurs (ne pas se prendre pour quelqu’un de meilleurs qu’eux, par exemple, que sais tu des contraintes de temps qu’ils avaient pour développer ce projet ?) c’est probablement encore plus important.