Le pire bout de code que vous ayez vu

Quand j'ai repris un projet sous C++ builder j'ai découvert dans chaque fonction du projet des déclarations de la forme :

Code:

---

1 2 3

entier a, b, c; chaine nom, prenom;

---

:koi:


Et après avoir longuement cherché (le projet possédait plus d'une centaine de fichiers) j'ai découvert dans un fichier C++ bien caché ceci :

Code:

---

1 2 3

typedef int entier; typedef AnsiString chaine;

---

Bref, chaque type C++Builder avait été francisé :aie:

Alors moi le pire morceau de code que j'ai vu a été écrit par une amie lorsque j'étais en BTS IG. C'était du VB.

Code:

---

1 2 3 4 5

for i=0 to 10 then if (je ne sais plus quel condition) then i = i -1 end if next i

---

Et le compilateur VB laissait passer ça !!! En Delphi, il est pas d'accord... Cette amie me demandait de l'aide parce que son programme plantait, quand j'ai vu ça, je lui ai dit : "bah tu as trouvé le moyen pas forcément évident de faire une boucle for infinie, donc tu vires ça et après je t'aide à trouver ton erreur". Elle était un peu vexée :mouarf:

J'ai vu aussi le if(true) dans Mitaka, un logiciel astronomique de 3D temps réel ............

Citation:

---

Envoyé par Acropole

Code:

---

1 2 3 4

if(1) { ...

---

---

Ce code là peut apparaitre assez naturellement dans deux situations...

1- en prévision d'un second cas, pour en marquer la place... (normalement il faudrait mettre un commentaire, quand même), ou pour pouvoir facilement éliminer la boucle ( if(0) )
2- après la suppression d'un else, un jour de presse, pour éviter des conflits de noms avec des variables locales au if()...

Ce n'est pas gracieux gracieux, mais comme ca disparait à la compilation, et que ca ne pose aucun problème de compréhension...

Francois

Citation:

---

Citation:

---

public void close(InputStream stream){
stream = null;
}

---

InputStream est un flux vers une ressource. Pour le fermer, par exemple, il faut invoyer sa méthode close(), mais dans ce cas, le "créateur" s'est contenté de le mettre à null. Les développeurs Java seront d'accord pour admettre l'inutilité crasse de ce bout de code : les variables sont passées par copie de la valeur, donc ce ne sera "null" que dans la méthode.

---

Je dis peut-être une bêtise (faut pas m'en vouloir, parfois je suis crevé après le boulot..) mais ce bout de code m'a l'air tout a fait fonctionnel.
En Java les paramètres ne sont passés par valeur que pour les types primitifs, pour les objets c'est par référence.
Ici donc c'est bien par référence que c'est passé.

Citation:

---

Envoyé par gibet_b

Et le compilateur VB laissait passer ça !!! En Delphi, il est pas d'accord...

---

je doute que Delphi refuse de compiler..mais si tu le dis alors....
Le code suivant

Code:

---

1 2 3 4 5

Dim i As Integer for i-0 To 10 i=i-1 Next i

---

est accepté par VB et en principe serait acceptée dans n'importe quel langage par n'importe quel compilateur..
en principe le compilateur ne fait pas d'analyse prédictive donc ce qui est correct sémantiquement parlant passe parfaitement à la compilation
Les compilateurs ne se préoccupent pas de savoir ce qui va se passer à la compilation et si tu dis qu'en Delphi cela passe je suis un peu sceptique.
Dans le même ordre d'idée en C++

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13

class ClasseQuelconque { void MethodeQuelconque(); }; int main() { ClasseQuelconque *pClasseQuelconque; pClasseQuelconque->MethodeQuelconque(); return 1; }

---

A la compilation cela passe sans problème , à l'exécution ..plantage

Mon passage préféré, c'est de l'AS3, mais c'est faisable dans plein d'autres langages...

Code:

---

1 2 3 4 5 6 7 8

public class SelectorItemVO { ... public var selectorItemsVO:SelectorItemsVO; public function SelectorItemVO(selectorItemsVO:SelectorItemsVO) { this.selectorItemsVO=selectorItemsVO; }

---

Pour ceux qui ont lu un peu vite, je résume...

On a une classe SelectorItemVO, qui définit les éléments (Item) d'un sélecteur (Selector) (le VO à la fin, c'est juste parce que ca fait plus pro...)

Les instances de cette classe ont pour vocation d'être intégrées dans des conteneurs, appelés SelectorItemsVO (notez le "s" après SelectorItem, c'est frappé au coin du bon sens : une collection d'item, c'est des items...)

Comme chaque élément doit connaitre le nom de son conteneur, il nous faut une variable membre, et la logique en impose le nom : selectorItemsVO (s minuscule au début, c'est la règle en AS3 comme dans d'autres langages)

Et bon, forcément, ce conteneur, il va falloir l'initialiser dans le constructeur, en lui passant une référence, appelée selectorItemsVO (comme ca on sait ce que c'est).

Ca part d'un bon sentiment: une convention de nommage forte, respectée scrupuleusement...

Francois

Citation:

---

Envoyé par xtope

Je dis peut-être une bêtise (faut pas m'en vouloir, parfois je suis crevé après le boulot..) mais ce bout de code m'a l'air tout a fait fonctionnel.
En Java les paramètres ne sont passés par valeur que pour les types primitifs, pour les objets c'est par référence.
Ici donc c'est bien par référence que c'est passé.

---

Non, en Java tout est par valeur... Tu passes une copie de la référence d'un objet, ce qui te permet de modifier les champs de cet objet mais en aucun cas sa référence. Et donc la mise à null ici est effectivement limitée au scope de la fonction.

Un singleton horrible!!!

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21

public class SingletonMoisi{ private static SingletonMoisi instance; private Context context; private SingletonMoisi(Context context) { this.context = context; } public synchronized static SingletonMoisi getInstance(Context context) { if(instance == null) { instance = new SingletonMoisi(context); } return instance; } public void faitUnTrucAvecTonContext() {...} }

---

Le premier qui parle fige le context, les suivant allez vous faire voir!
Je m'explique:

Code:

---

1 2 3 4 5 6

// Ici on utilise le context1 SingletonMoisi.getInstance(context1). faitUnTrucAvecTonContext(); // Ici on veut utiliser le context2 mais c'est le 1 qui sera utilisé, domage SingletonMoisi.getInstance(context2). faitUnTrucAvecTonContext();

---

Les singletons je les préfères comme ca:

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12

public class Singleton{ private final static Singleton INSTANCE = new Singleton(); private Singleton(){ } public static Singleton getInstance() { return INSTANCE; } }

---

Vu à plusieurs reprises, notamment une fois dans un template de magento :

Code:

---

1 2 3 4 5 6 7

<?php if(condition1) { //gros bloc de HTML (genre 20 à 30 lignes) } else if(condition2) { //le même gros bloc, à 1 ligne, voir 1 variable près } else //et ainsi de suite ?>

---

Ca va pas faciliter la maintenance :calim2:


Mais le pire que j'ai vu, c'est quand même un site qu'une grosse agence m'a refilé pour le débuguer et le terminer, où chaque page était un imbroglio de PHP (avec des if() dont on ne trouvait plus la fin du bloc), SQL (avec des requètes kilométriques de 3 ou 4 UNION), HTML (à base de <table>imbriqués), JS (généré par dreamweaver), et un peu de CSS à droite à gauche, histoire de ...

La notion de pire est à relativiser, c'est à l'appréciation de chacun et de son niveau d'expérience dans la programmation.

On ne peut pas blâmer un gamin de 14 ans écrire (ou copier/coller) du PHP pour son site internet ou bien un "stagiaire" qui débute en java ou autre langage. L'important est de les guider vers les bonnes pratiques.

Selon moi, en premier lieu, le pire code est celui qui ne correspond pas au but souhaité (non conformité).
En second lieu, vient la manière d'écrire le besoin qui ne respecte pas les principes de base de l'algorithme, de la philosophie du langage et ses bonnes pratiques.

N'oublions pas que c'est en forgeant qu'on devient forgeron ;)

Je ne peux pas justifier de nombreuses années d'expérience, mais cela ne m'a pas empêché d'en voir de belles.
Celle dont je me souviendrais longtemps, c'est un site développé en PHP dans lequel sur à peine dix lignes on trouvait l'équivalent de ceci :

Code:

---

1 2 3 4 5 6 7

$result = mysql_query(' SELECT truc, bidule FROM ma_table WHERE champ = '.$_POST['valeur'].' AND autre_champ = '.$_COOKIE['mon_cookie'].' LIMIT '.$_GET['debut'].', 10 ') or die(mysql_error());

---

Pour l'injection SQL, je vous laisse choisir la méthode que vous préférez :D

Bonjour,

Citation:

---

Envoyé par Gordon Fowler

Tout cela est faisable en une seule et simple ligne de Java (avec Integer.parseInt(String value) ).
Mais le pire c'est que, en plus, la méthode de cette autre "créateur fou" ne fonctionne pas du tout. En Java, on ne peut pas faire value == "1" pour comparer sa valeur (il faut faire "1".equals(value) ).

---

Pas tout à fait. Ce qui est important je pense (et très intéréssant) c'est qu'un débutant en viendra à écrire ce genre de code mais de manière itérative. Il va se retrouver confronté à un problème puis en résoudre une partie etc jusqu'a avoir un code qui marche mais ... un peu dégeulasse. Avec le switch on voit clairement que le débutant à perdu son objectif principal, et a très certainement été confronté au fait qu'en java un string ne passe pas dans un switch (bien évidement même avec un switch ce code reste inutil mais au final ce qui est dégeulasse c'est pas de recoder la roue mais de la recoder mal). Quand on est pas dans un context d'entreprise, recoder la roue est d'une richesse infinie. Pour contourner le problème des strings avec les switch je conseil de lire ce billet.

Par contre c'est faux de dire que == ne marche pas il a juste un comportement un peu spécial. Il ne compare pas les valeurs mais les adresses en mémoire. Le fait est que dans ce cas préci pour peu que l'on passe un litéral à la méthode convert, le programme marchera normalement. En effet la JVM implémente le pattern flyweight qui construit un parc de données pour les littéraux. De ce fait les littéraux sont partagés dans tous le programme. Puisqu'ils sont partagés alors quand on fait "1" == "1", alors on compare les deux adresses, qui pointent vers la même instance en mémoire, et qui du coup est égal. En fait on fait un test qui ne fait pas ce qu'on veut mais qui au final donnera le bon résultat. Pour être certain que le code marche il faudrait non pas utiliser value mais value.intern(). Pour plus de détails pous pouvez lire ce billet.

Concernant le débat je ne participerais pas au "cassage de noob". On y est tous passé, et ca sert à rien de les pointer du doigt. Je préfère voir quelqu'un qui écris un mauvais code que quelqu'un qui ne code pas.

Bonjour,

Citation:

---

Envoyé par monsieurben

J'ai bossé avec un presta tellement nul que je sais pas trop quel bout de code choisir.

J'ai eu droit à celui-ci, plutôt pas mal :

Dans une page de traitement d'un formulaire :

Code:

---

1 2 3 4 5

<?php $nom = post('nom'); $prenom = post('prenom'); [...]

---

Et dans un fichier inclus, ladite fonction :

Code:

---

1 2 3 4

<?php function post($var) { return $_POST['$var']; }

---

La fonction ne servait strictement à rien, mais elle était consciencieusement appliquée à tous les champs de tous les formulaires (plus de 500 champs)...

---

C'est une facon de cacher l'implémentation. Le fait de créer cette indirection n'est pas une mauvaise pratique au contraire. Ici on voit que ca ne sert à rien, mais ca évite de refactorer ton code une fois que tu auras des modifitations à faire au moment de la récupération de tes champs post. A ce moment là tu va finir par faire pareil que lui pour factoriser ton code, sauf que tu l'auras pas prévu à l'avance. Là au moins c'est tout prêt, lui l'a prévu ...

J'en ai vu seulement quelques-uns du fait de ma courte expérience. Le plus marquant, il y a 2 ans, je devais reprendre une application débutée un an plus tôt par un autre stagiaire, développée en C++ .net. Application desktop adossée a une base de données, gestion de rapports, pas mal de lectures et mises a jour de la base. Le probleme c'est qu'a chaque requete faite a la base de données, mon prédécesseur avait simplement copie-colle les ~25 lignes de connexion a la base. C'est-a-dire qu'a chaque fois qu'il fallait récupérer un champ de la base de données pour l'afficher, hop, on collait ce bout de code. Idem quand on le mettait a jour. Au final, je me retrouvais avec des fichiers de plusieurs milliers de lignes, dont les 25% étaient des lignes identiques de connexion a la base :calim2::calim2::calim2:

Code:

---

$ cat file | wc -l

---

:ccool:

Citation:

---

Envoyé par gibet_b

Alors moi le pire morceau de code que j'ai vu a été écrit par une amie lorsque j'étais en BTS IG. C'était du VB.

Code:

---

1 2 3 4 5

for i=0 to 10 then if (je ne sais plus quel condition) then i = i -1 end if next i

---

---

une façon esthétique de maintenir la boucle tant qu'on a pas esquivé 10 fois le if ?

Citation:

---

Envoyé par Mat.M

je doute que Delphi refuse de compiler..mais si tu le dis alors....
Le code suivant

Code:

---

1 2 3 4 5

Dim i As Integer for i-0 To 10 i=i-1 Next i

---

est accepté par VB et en principe serait acceptée dans n'importe quel langage par n'importe quel compilateur..
en principe le compilateur ne fait pas d'analyse prédictive donc ce qui est correct sémantiquement parlant passe parfaitement à la compilation

---

Le compilateur Delphi interdit la modification de la variable utilisée par une boucle for.

Vu dans un programme en production en Java (et pas qu'une fois) :

Code:

---

1 2 3 4 5 6 7 8 9 10

int[] tablo = new int[10]; int i = 0; try { while (true) { tablo[i] = 0; i++; } } catch (ArrayIndexOutOfBoundsException iobe) { }

---

Réponse du développeur : j'ai lu un article sur le Web qui dit que c'est plus rapide de faire comme cela :D

Du même développeur, vu dans un javadoc :

/**
* TODO : faire que ça marche
*/

Véridique !

Laurent

Un petit bout de code Java vu il y a quelques temps...

Code:

---

1 2 3 4

for (int i = 0; i < laListe.size(); i++) { Object o = laListe.iterator().next(); ... //code divers }

---

Création d'un itérateur à chaque tour de boucle, pourquoi pas... le résultat est juste un peu monotone :mouarf: