Le pire bout de code que vous ayez vu

**kiwwi209** · 06/11/2009, 00h04

une trouvée lors d'une revue de code cet après-midi en Java :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
try {
     // un bout de code (1)
} catch (Exception e) {
     // exactement le même bout de code que (1) avec d'ailleurs la même faute de frappe dans le commentaire !!
}

y en a chez qui il va falloir reprendre les bases de la gestion des exceptions...

Invité · 06/11/2009, 04h50

Envoyé par kiwwi209

une trouvée lors d'une revue de code cet après-midi en Java :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
try {
     // un bout de code (1)
} catch (Exception e) {
     // exactement le même bout de code que (1) avec d'ailleurs la même faute de frappe dans le commentaire !!
}

y en a chez qui il va falloir reprendre les bases de la gestion des exceptions...

En effet, aucun try-catch imbriqué dans le premier catch, au cas ou une exception soit levée... Honteux

**jabbounet** · 06/11/2009, 09h22

Envoyé par PierreAd

En effet, aucun try-catch imbriqué dans le premier catch, au cas ou une exception soit levée... Honteux

Du même tonneau dans le même fichier source 2 méthodes aux signatures et au code identique (idem à l'octet près) et au nom différent l'une au dessus de l'autre, les deux étant utilisé un peu partout dans le code sans règles précises.

**bého32** · 06/11/2009, 11h26

Un membre de mon équipe projet qui apporte ses modifications :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
if (myInstance != null)
{
    return myInstance;
}
else
{
    return null;
}

**jabbounet** · 06/11/2009, 11h39

Envoyé par bého32

Un membre de mon équipe projet qui apporte ses modifications :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
if (myInstance != null)
{
    return myInstance;
}
else
{
    return null;
}

il a l'avantage de ne pas planter, même si c'est un peu bebête, ceci dit il serait bon de lui dire que le if / else ne sert a rien ici.

**ronan99999** · 06/11/2009, 13h47

Je déteste les goto.... ici en c++

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
bool function()
{
  bool test;
  /*....*/
  if(!b)
   goto error;

end:
 return test;
error:
  test = false;
  goto end;
}

Avec toujours les mêmes excuses:
"Mais les exceptions ça consomme trop de temps"
"Le goto est propre, ça évite d'avoir trop d'indentation" (autant que le nucléaire)

Je mettrais dans le même sac les c-style cast....Et la RTTI c'est fait pour décorer!!!!!

Marre du C--.

ps: un peut énerver je viens de tomber la dessus.

**covao** · 06/11/2009, 13h52

L'effet ravageur d'une simple instruction de gestion d'une facture

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
...
DELETE FROM LignesFactures WHERE Position_Ligne > :PositionLigne;
...

**AnneCa** · 06/11/2009, 15h02

Un bel exemple en SQL. On a une table TABLE1 (Champ1, Champ2) où Champ2 est numérique.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
DECLARE Cursor1 CURSOR FOR SELECT  Champ1 FROM TABLE1

OPEN Cursor1 
FETCH Cursor1 INTO @cVAR 
WHILE (@@Fetch_Status = 0)
BEGIN
   UPDATE TABLE1 SET champ2 = champ2 + 1
   WHERE Champ1 = @cVAr 
   FETCH Cursor1 INTO @cVAR 
END

Une belle longue boucle alors qu'une requête suffit

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

UPDATE TABLE1 SET Champ2 = Champ2 +1

**deadalnix** · 06/11/2009, 19h27

Aujourd'hui, j'en ai vu une belle !

Sur une appli web, il y a deux façon de se loguer. La façon classique et au autologin depuis un lien spécial.

Le programmeur qui s'était occupé de l'autologin avait simplement fait géré une formulaire de login préremplis (avec le pass en clair et tout) qui se soumet automatiquement vers la façon de se logguer n°1.

**apieum** · 07/11/2009, 05h42

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
<?php 

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

Et dans un fichier inclus, ladite fonction :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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)... .

Je pense pas que ce soit une mauvaise pratique.
C'est plus simple d'écrire post('prenom') que de taper un underscore et POST en majuscule, mais surtout, c'est un hook qui permet de modifier rapidement les variables post des 500 champs.

par exemple:
si tout d'un coup on décide de n'avoir que des chaines en minuscule.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
<?php
function post($var) {
return strtolower($_POST["$var"]);
}

http://fr.wikipedia.org/wiki/Hook_(informatique)

**OWickerman** · 07/11/2009, 07h35

Le genre de truc que je déteste en PHP, l'utilisation abusive des echo...

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
	<?php
			include ('fonction.inc');
			titre('Pièces justificatives');
		echo "<table border='0' width='95%' CELLPADDING='1' align='center' class='txt_tab_tarif'>";
		echo "<tr><td colspan='2'>";sstitre('Joindre les pièces justificatives suivantes');echo "</td></tr>";
		echo "<tr><td colspan='2'>(<b>photocopie</b> obligatoirement):</td></tr>";
		echo "<tr><td colspan='2'>&nbsp;</td></tr>";
		echo "<tr><td><img src='img/puce2.gif'></td><td>Photocopie du justificatif d'adresse (Eau, EDF...)</td></tr>";
		echo "<tr><td valign='top'><img src='img/puce2.gif'></td><td>Photocopie de l'avis d'imposition ".(date('Y')-2)." ou ".(date('Y')-1)." (si vous êtes imposable)<br> OU photocopie de la dernière notification de la CAF (".date('Y')." avec le quotient familial)</td></tr>";
		echo "<tr><td><img src='img/puce2.gif'></td><td>Cachet de l'établissement scolaire pour les primaires, maternelles et collégiens dans le cadre réservé à cet effet.</td></tr>";
		echo "<tr><td><img src='img/puce2.gif'></td><td>Attestation d'inscription dans l'établissement scolaire pour les élèves fréquentant un lycée</td></tr>";
		echo "<tr><td colspan='2'>&nbsp;</td></tr>";
		echo "<tr><td colspan='2' align='center'><a href='#'><img src='img/b_fermer.gif' border='0' onClick='self.close();'></a></td></tr>";
		echo "</table>";
	?>

Cette page la étant simple, ça va encore... J'en ai de pires :
Celui ci, par exemple, qui, non seulement abuse des echo, mais en plus essaie de sauvegarder des sauts de lignes....
Ah, oui, la technique des tableaux dans des tableaux est un truc de fourbe. On devrait pendre les gens qui osent faire ça !

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
			echo "<form name='rech_iti' id='rech_iti' method='GET' action='rech_iti_res.php' target='_self'>";
			echo "<table border='0' width='95%' CELLPADDING='0' align='center' class='txt_tab_tarif'>";
			echo "<tr><td colspan=2 class='txt_sstitre'>Merci de remplir le formulaire suivant</td></tr>";
			echo "<tr><td colspan=2 class='txt_sstitre'>&nbsp;</td></tr>";
			echo "<tr><td>Période de départ souhaité</td><td>";
			echo "<select name='list_per' class='txt_menu_der' id='list_per'>";foreach($tabperiode as $cle=>$valeur){echo "<option class='txt_menu_der' value='".$cle."'>".$valeur."</option>\n";} 
			echo "</select>";
			echo "</td></tr>";
			echo "<tr><td>Heure</td><td>";echo "<select name='heure' class='txt_menu_der' id='heure'>";for($i=0;$i < 24;$i++){echo "<option class='txt_menu_der' value='".$i."'>".$i."</option>\n";}echo "</select> &nbsp; h &nbsp;&nbsp;";echo "<select name='minute' class='txt_menu_der' id='minute'>";for($i=0;$i <= 59;$i{ echo "<option class='txt_menu_der' value='".$i."'>".$i."</option>\n";}echo "</select>&nbsp;&nbsp;mn";
			echo "</td></tr><tr><td colspan='2'>&nbsp;</td></tr>";
			
			//DEPART
			echo "<tr><td colspan='2'>";
			echo "<fieldset style='border:2px solid rgb(191, 191, 191);width: 400px'>";
				echo "<legend class=txt_sstitre>Départ</legend>";
				echo "<table border='0' align='center' width='80%' CELLPADDING='0' class='txt_menu_der'>";
				echo "<tr><td>Commune</td><td>";chmpderoul_noSel_free('list_comm_dep','txt_menu_der',$tabcommune,"onchange=\"go('php_ajax/commune_dep.php','')\"");echo "</td></tr>";
				echo "<tr><td>Rue</td><td>";chmpderoul_mess_free('list_rue_dep','txt_menu_der',$tabrue_dep,'Choisir une commune',"onchange=\"go('php_ajax/rue_dep.php','')\"");echo "</td></tr>";
				echo "<tr><td>Station</td><td>";chmpderoul_mess_free('list_station_dep','txt_menu_der',$tabstation_dep,'Choisir une rue',"");
				echo "<div name='blocdepart' id='blocdepart'><input type='hidden' name='sta_dep' id='sta_dep' value='0'></div>";
				echo "</td></tr>";
				echo "</table>";
			echo "</fieldset></td></tr><tr><td>&nbsp;</td></tr>";
			
			//ARRIVEE
			echo "<tr><td colspan='2'>";
			echo "<fieldset style='border:2px solid rgb(191, 191, 191);width: 400px'>";
				echo "<legend class=txt_sstitre>Arrivée</legend>";
				echo "<table border='0' align='center' width='80%' CELLPADDING='0' class='txt_menu_der'>";
				echo "<tr><td>Commune</td><td>";chmpderoul_noSel_free('list_comm_arr','txt_menu_der',$tabcommune,"onchange=\"go('php_ajax/commune_arr.php','')\"");echo "</td></tr>";
				echo "<tr><td>Rue</td><td>";chmpderoul_mess_free('list_rue_arr','txt_menu_der',$tabrue_dep,'Choisir une commune',"onchange=\"go('php_ajax/rue_arr.php','')\"");echo "</td></tr>";
				echo "<tr><td>Station</td><td>";chmpderoul_mess_free('list_station_arr','txt_menu_der',$tabstation_dep,'Choisir une rue',"");
				echo "<div name='blocarrivee' id='blocarrivee'><input type='hidden' name='sta_arr' id='sta_arr' value='0'></div>";
				echo "</td></tr>";
				echo "</table>";
			echo "</fieldset></td></tr>";
			
			echo "<tr><td colspan='2'>&nbsp;</td></tr><tr><td colspan='2'>&nbsp;</td></tr>";
			echo "<tr><td colspan='2' align='center'><img src='img/b_rechercher.gif' style='cursor:pointer;' border='0'  onClick='testEtValid(\"rech_iti\");'>";
			
			 echo "</td></tr>";
			echo "</table></form>";
		?>

Pour infos, quand on m'a demandé de finaliser le site, j'ai du remettre les pages au propre avant de comprendre ce qui s'y passait...

**nicolofontana12** · 07/11/2009, 12h32

Il y'a certain code qui ne fait pas seulement pleurer, ni rire! Surtout lorsque ton chef de service le fait!

**mixouille** · 07/11/2009, 15h24

Vu il y a peu de temps :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
public class Machin extends Truc {

   [...]

   public boolean equals() {
      return this.equals();
   }
}

Le but était peut-être de faire un super.equals(), ce qui au moins aurait évité le plantage, bien qu'étant parfaitement inutile.

screetch · 07/11/2009, 16h17

je travaille dans les jeux, et souvent on a un besoin de perf assez fort. Aussi, dans le temps (il y a un bail, avant l'arrivée de la FPU) il existait des calculs un peu magiques pour approximer des opérations un peu complexes (je pense notemment a l'inverse de la racine carrée de Carmack :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
float InvSqrt (float x){
    float xhalf = 0.5f*x;
    int i = *(int*)&x;
    i = 0x5f3759df - (i>>1);
    x = *(float*)&i;
    x = x*(1.5f - xhalf*x*x);
    return x;
}

ce code est désormais moins performant que l'original, car o a désormais une FPU (et surtout des instructions SSE) et ce code force a passer par la mémoire, ce qui bloque le CPU, ce qui coute au final aujourd hui plus cher que l'opération originale. Enfin bon bref, a cette époque ce bout de code était incroyable.

On a aussi inventé vers cette epoque les tables de cosinus, ou on précalculait le cosinus de tous les angles que l'on mettait ensuite dans une table, pour ne pas avoir a faire l'opération.

et chez nous (pourtant, c'était des _années_ apres l'invention de la FPU et les CPU courraient déjà a plus de 2GHz) pour accélerer, on avait aussi une table précalculée, remplie comme suit

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
for(int i = 0; i < 32; ++i)
{
  tab[i] = 2**i; /* pseudo code, c'était du pascal */
}

un tableau des puissances de deux, pour accélerer le jeu.

...

2 puissance x en entier se calcule en un cycle CPU

**deadalnix** · 07/11/2009, 18h36

J'en connais un qui ne connaissait pas bien les décalages

**goofyto8** · 08/11/2009, 17h51

10 a$=inkey$
20 if a$ ="" then goto 10
30 print
40 end

**nicolofontana12** · 09/11/2009, 02h03

Souvent il est tres difficile d'etre chef de projet. Surtout si le chef lui meme essaye essaye de try System.out.println(""); qu'est ce qu'on va mettre alors dans catch

**Fredouille91** · 09/11/2009, 09h16

Il n'est pas si idiot que ça ce code, au contraire.
Imaginons qu'un jour on doivent appliquer un traitement à toutes les valeurs postées (par ex, un test d'injection SQL, une mise par défaut à 0 de valeurs numériques,...)...

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
<?php 

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

Et dans un fichier inclus, ladite fonction :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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)...

J'en ai plein d'autres comme ça :

- Le CMS utilisait adodb pour accéder à la BDD. Par dessus, une classe d'abstraction stockée dans un Zend_Registry, et par dessus un ensemble de fonctions style dbQuery(), dbResult() pour faire abstraction de la classe d'abstraction.

L'agence est une très grosse webAgency parisienne.

Invité · 09/11/2009, 09h55

A la fac j'ai été en binôme sur un projet avec quelqu'un qui avait réécrit la fonction malloc, car celle de base n'était pas assez "optimisée".
Ca m'avait bien fait rire.

Et puis dans une boîte j'ai rencontré quelqu'un qui a écrit ça :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
void
my_strcpy1 (const char *ch1,UCHAR *ch2)
{
	while(1)
	{
		if(!*ch1)
			break;
		*ch2++ = *ch1++;
	}
	*ch2= 0;
}

void
my_strcpy2 (UCHAR *ch1,UCHAR *ch2)
{
	while ((*ch2++ = *ch1++)!=0)   ;
	ch2++;
}

On appréciera :
- le fait de réécrire des fonctions standard
- le fait de les réécrire DEUX fois
- l'ordre des paramètres inversé par rapport à la fonction standard
- cette magnifique façon d'écrire un while dans la première fonction

**jpelaho** · 09/11/2009, 10h32

Envoyé par crazyday

J'en ai vu un pas mal en php, fait par le responsable développement de ma boîte :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
if (true) {
} else {
...
}

Ce jour-là je me suis dis que finalement je n'étais pas le plus mauvais ...

Juste pour réagir par rapport à ce genre de code que je ne trouve pas si idiot que ca. Genre if(true) ou if(1).

Il m'est déjà arrivé de faire ca dans mon code. La raison c'est qu'il ya effectivement un test qui doit être mis à ce niveau, je fais le test mais il s'avère que ce n’est pas le bon test alors je l'enlève mais sans supprimer l'instruction if (cela me permet de me rappeler cela plus tard en voyant un "if" qui n'a pas de sens dans le code).

Bien entendu pour faire une release, il faut enlever le test ou le corriger. Mais ca peut souvent aider de laisser ca dans un code de façon momentané.

Le pire bout de code que vous ayez vu

Discussions similaires

Partager

Partager