Discussion :

[Claude30120]

Bonsoir
Dans mon développement, j'ai utilisé les variables dynamiques pour centraliser le développement.
Car, le travail que j'ai à faire, est sensiblement le même pour créer une commande ou une facture.

Depuis que j'ai implanté cela en production, les utilisateurs constatent un net ralentissement.
Est-ce que quelqu'un peut valider que l'utilisation des variables dynamiques plombent les temps de réponse?

Cordialement
Claude

[CosmoKnacki]

Difficile de te dire ce qui plombe vraiment ton code sans avoir le dit code sous les yeux. Ça m'étonnerait que les variables dynamiques en elles-mêmes y soient pour quelque chose.

[Claude30120]

Ma question était générale.
CosmoKnacki, votre question m'interroge, est-ce qu'un code mal écrit peut ralentir l'exécution? Si oui, avez-vous qeulques exemples?

Si vous avez le temps, voilà une partie du code où j'utilise tant et plus les variables dynamiques.
Il y a plus de 200 lignes de code. Si vous n'avez pas le temps, je le comprendrai bien.

Attention, pour ceux qui liraient ce code à des fin de formation, il n'est absolument pas écrit dans les canons de l'art.
Cordialement
Claude

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
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
 
function Traite_cmd_fact_complete ($nomclasse, $bull)
	{
		global $user, $langs;
		global $gl_error; // utilise dans FactureBulletin
		$retour = 0;
		$wc1 = New CglCommunLocInsc($this->db);
 
		if ($nomclasse == 'Facture') {
				$nomclassedet = 'FactureLigne';
				$fk_champdet = 'fk_linefct';
				$fk_champ = 'fk_facture' ;
		}
		elseif ($nomclasse == 'Commande') {
			$nomclassedet = 'OrderLine';
			$fk_champdet = 'fk_linecmd';
			$fk_champ = 'fk_cmd' ;
		}
		$v_nomclasse=strtolower($nomclasse);
		$fk_objet = 'fk_'.$v_nomclasse;
		$fk_objetdet = 'fk_line_'.$v_nomclasse;
		$ref_objet = 'ref_'.$v_nomclasse;
		$tabledet = $v_nomclasse.'det';
		$flg_creation = false;
		$objet = new $nomclasse($this->db);		
		if (empty($bull->{$fk_objet})  or $bull->{$fk_objet} <=0)		{
			$flg_creation = true;
			// facture ou commande à créer 
			$this->interface_bull_cmd_fact($fk_objet,  $bull, $objet, $nomclasse, $nomclassedet);
			$ret = $objet->create($user, 0);
			if ($ret <0) {
				$gl_error = $ret;
				return  -1;
			}
			// Recupération des clés étrangères
			$bull->{$fk_objet} = $objet->id;
			$bull->update_champs($fk_champ,$objet->id);
			// recherche des identifiant des lignes de commandes
			foreach ($bull->lines as $bullline)			{
				if ($bullline->action != 'X' and $bullline->action != 'S' and ($bullline->type_enr == 0 or $bullline->type_enr == 2 ) )	{
					// activite ou materiel loue
					$idobjdet = $bullline->rechercheIdCmdFactDet ($tabledet, 'fk_'.$v_nomclasse, $objet->id);
					$bullline->{$fk_objetdet} = $idobjdet ;	
					$bullline->update_champs($fk_champdet,$idobjdet );
				}
			}
			$wc1->MajVentilationFactDet($bull->{$fk_objet});
			$retour = 1;
			// lien Dolibarr Commande-facture  Bulletin
			$this->AjoutElement($bull->id, $bull->{$fk_objet}, $v_nomclasse);				
		}
		else {
			// facture existante à mettre à jour
			$objet->fetch($bull->{$fk_objet});
			$objbroullon = $objet->brouillon;
			$objet->brouillon = 1;
			$objet->statut = 0;
			//$objet->id = $bull->{$fk_objet};
			foreach ($bull->lines as $bullline)		{				
				if ($bullline->type_enr == 0 or $bullline->type_enr == 2 ) 
					$ret = $this->Traite_ligne_cmd_fact( $bullline, $nomclasse,$nomclassedet, $fk_objet, $fk_objetdet, $fk_champdet , $objet, $bull);
				if ($ret < 0) return -1000+$ret;
			}			
			$objet->brouillon = $objbroullon ;
		}
		// mettre à jour le total dans la facture
		$ret = $objet->update_price();
		if ($ret < 0) return -2;
 
 
		$ret = $objet->valid($user,0);		
		if ($ret >0) 		{
			$bull->action == '' ;
			$bull->updateaction ('');
		}		
		unset ($objet);			
		return $retour;	
 
		} //Traite_cmd_fact_complete
function interface_bull_cmd_fact_entete($fk_objet,  $bull, $objet, $nomclasse)
		{
		global  $langs;	
		// entete de fature/commande
		$objet->socid = $bull->id_client;		
		$objet->ref_ext =  $bull->ref;
		$objet->date = dol_now();
		//$objet->date = dol_stringtotime($bull->datec);
		$objet->ref_client = $bull->ref_client;
		$objet->demand_reason_id = $bull->fk_origine;
		$objet->modelpdf = $bull->RechercheModelFactCmd($nomclasse);
		if ($bull->type == 'Insc') $objet->note_private = $langs->trans('Bulletin') ;
		else $objet->note_private = $langs->trans('Location') ;
		$objet->note_private .= ' : '.$bull->ref;		
 
		if ($bull->type == 'Insc') $note_public = $langs->trans('Bulletin');
		else  $note_public = $langs->trans('Contrat ');
		$objet->note_public.=':'.$bull->ref;
 
		$objet->brouillon 		=	1;
		$objet->statut 		=	0 ;
	} //interface_bull_cmd_fact_entete
 
 
function Traite_ligne_cmd_fact( $bullline, $nomclasse, $nomclassedet,  $fk_objet, $fk_objetdet, $champdet, $objet, $bull)
	{
		global   $user, $langs;
		global $gl_error_fk; // Utiliser dans this->FactureBulletin 
		global $gl_facture; // Utiliser dans this->FactureBulletin 
 
		$wc1 = New CglCommunLocInsc($this->db);
 
		$retour = 0;			
		$flnvobj = false;
		if (empty($objet)) {
			$flnvobj = true; 
			$objet  = new $nomclasse($this->db);
			$ret = $objet->fetch($bull->{$fk_objet});
			if ($ret < 0) {
				unset ($objet);
				$gl_error_fk = $bull->{$fk_objet};
				return -1;
			}
		}
		$gl_facture = $objet->ref;
		/* Mettre le statut de la commande à 0 pour pouvoir faire les delete, add et update de ligne */		
		if ($flnvobj) { 
			$objet->statut = 0;
			$brouiollonsav = $objet->brouillon;
			$objet->brouillon = 1;
		}		
 
		if ($bullline->action == 'S'    or  $nomclasse == 'Facture' )		{
			// supprime ligne commande
			$ret = $objet->deleteline($bullline->{$fk_objetdet});
			if ($ret <0 ) {			
				if ($flnvobj) unset ($objet);	
				$gl_error_fk = $bullline->id;
				return -2;
			}
			$bullline->{$fk_objetdet} = 0;
			$ret = $bullline->update_champs($champdet,0);
		}
		else 	{
			if ($bullline->type_enr == 0) {
				$tva = $bullline->taux_tva;
				$pu =  price2num($bullline->pu)*100/(100 + $tva);
				$pt = $bullline->pu;
				if ($tva == 0) 	$typetva='TTC';
				else 	$typetva='HT';					
				$label = ($bull->type == 'Loc') ? $this->LabelCmdFactDetLoc ($bullline): $this->LabelCmdFactDet ($bullline);	 
			}
			elseif ($bullline->type_enr == 2) { 
				if ($bullline->mttremfixe == 0 ) return;
				$tva = $bull->TauxTVARemiseFixe();
				$typetva='TTC';
				$pu = price2num(0 - $bullline->mttremfixe);	
				$label = $bull->lbremfixe;	
				$pt = $pu;
				$bullline->qte = 1;
				$bullline->rangdb = 99;				
			}
 
			if (empty($bullline->{$fk_objetdet}) )	{
				// transformer le code ventilation en rowid
				$codeventilation = $wc1->RechIdVentilationbyCode($bullline->fk_code_ventilation,  $bull->type);
				if ($bull->type == 'Loc') $fk_produit = $bullline->activite;
				else $fk_produit = $bullline->fk_produit;
					$ret = $objet->addline($label, $pu, $bullline->qte,$tva,0,0,$fk_produit,	$bullline->remise_percent,'','',$codeventilation,'', '',$typetva, $pt,1, $bullline->rangdb,0,0,null,0,'');
				if ($ret < 0) {
					if ($flnvobj) unset ($objet);
					$gl_error_fk = $bullline->id;
					return -100 + $ret;
				}
				else {		
					$bullline->{$fk_objetdet} = $ret;
					/*$ret = $objet->fetch($bull->{$fk_objet});
					if ($ret < 0) {
						if ($flnvobj) unset ($objet);
						return -120 + $ret;
					}*/
					$ret = $bullline->update_champs($champdet,$bullline->{$fk_objetdet});
				}	 // Suite OK de addline
			} // Fin de creation
			else {
				if ($bullline->type_enr == 2) 
					$ret = $this->MajPrixCmdFactDet($bullline->{$fk_objetdet}, price2num($pu), price2num($pt), $label,   $nomclasse, $tva);
				elseif ($bullline->type_enr == 0) 
					$ret = $objet->updateline($bullline->{$fk_objetdet}, $label, $pu,  $bullline->qte,$bullline->remise_percent,$tva,0,0,$typetva,0,'','',1,0,0, null,0,'',0);
				if ($ret < 0) {
					if ($flnvobj) unset ($objet);
					$gl_error_fk = $bullline->id;
					return -140 + $ret;
				}
 
				//taitement de la ventilation 
				$codeventilation = $wc1->RechIdVentilationbyCode($bullline->fk_code_ventilation,  $bull->type);
				if ($nomclasse == 'Facture') $ret = $wc1->updatefacventilation( $bullline->{$fk_objetdet} ,$codeventilation);
			}// Fin de MAJ
 
 
		} // Fin de ligne de bulletin à traiter	
		if ($flnvobj) {		
			// Remettre à jour le total de la facture
			$ret = $objet->update_price();
			if ($ret < 0)  {
				if ($flnvobj) unset ($objet);
				return -160;
			}		
			$ret = $objet->valid($user,0);
			unset ($objet);
			if ($ret <= 0)  {
				if ($flnvobj) unset ($objet);
				return -180 ;
			}
		}
		if ($ret >= 0 ) {
		}
		unset($wc1);
		return $retour;		
	} // Traite_ligne_cmd_fact

[CosmoKnacki]

Un code comme celui-là est très difficile à déboguer car:

• il n'y a pas de convention de nommage (on passe du français à l'anglais et vice-versa, de l'abrégé au mot complet, du camel-case au snake-case au tout agglutiné en lowercase). Tu dois en choisir une, une fois pour toute et t'y tenir. Le but du jeu est qu'une personne étrangère au code puisse immédiatement saisir ce que représente chaque élément (variable, fonction, attribut, méthode, nom de classe), ce qu'il contient, ce à quoi il sert et comment il doit être écrit.
• il n'y a pas de cohérence dans l'indentation, les sauts de lignes, les espacements. Ça ne facilite pas la lecture ni le débogage. (Tu peux prendre exemple sur les conventions adoptées par les frameworks, les moteurs de blogs, les codes sur github...)
• les fonctions sont trop longues et contiennent trop de variables différentes. Peut-être faut-il déléguer certaines tâches à de nouvelles fonctions de manière à ce qu'elles restent courtes. Donc n'hésite pas à découper.

Tu dis que les bulletins de commande et les factures ont beaucoup de point communs. Dans ce cas utilise la POO. Tu peux créer une classe abstraite avec tous les éléments communs aux deux, puis créer deux classes qui en héritent avec chacune leurs particularités (c'est un design possible, il y en à d'autres, à toi de réfléchir dessus).

Ce que j'ai aborder demande du temps et un retour en arrière, mais pour résumer l'idée, sans un code clair et bien conçu, à un moment ou à un autre tu risques de rendre cauchemardesque le moindre changement ou débogage. Au final tu risques de perdre plus de temps à t'acharner sur ce code en l'état.

[Celira]

Et sinon, pour répondre à la question d'origine : tout dépend de ce que "mal écrit" veut dire. En gros, il faut différencier le code "moche" (par exemple, ce que décrit Cosmo : mal indenté, des variables nommés en dépit du bon sens...) et un code "mal conçu".
En général, un code moche marche bien, mais il est illisible par un humain autre que celui qui l'écrit (et souvent également par celui qui l'a écrit quand il le reprend 6 mois plus tard )
Un code mal conçu, c'est un code qui fait des choses inutiles, ou qui les fait plusieurs fois, ou qui s'y prend comme un pied, et là, ça peut être dangereux pour les performances.

Exemple : j'ai une table qui contient des inscriptions à un événement, avec une colonne qui contient une référence vers l'événement, un nom et un "nombre de participants" par inscription.
Je veux afficher les 5eres inscriptions et le nombre total de participants.

• Ce qui est une bonne base : 2 requêtes en base : une pour récupérer la somme des nombres de participants à l'événement et une pour récupérer les 5eres lignes de la table pour l'événement.
• Ce qui peut améliorer : Un index sur la colonne ref_evenement de la table en base. Récupérer d'abord le nombre total de participants et tester si il est différent de 0 avant de faire la 2e requête.
• Ce qu'il ne faut pas faire : Récupérer toutes les inscriptions à l'événement, faire la somme des participants en PHP et ensuite tronquer la liste aux 5eres lignes.

Pour ton cas particulier, je ne pense pas que l'usage de variables dynamiques soit un problème (à part pour la lisibilité), mais je dois avouer que je n'ai rien compris à ton code, et que donc je n'ai réellement de piste à te donner

[Claude30120]

Bonsoir

Je vous dis un grand merci de vous être penché sur ce code, j'avais bien la prémonition des réactions qu'il a suscité.
C'est vrai que je ne travaille pas avec toute la sérénité voulue, remettant à plus tard, la clarification du code, que j fais de temps en temps, quand l'occasion se présente et que j'ai le temps (je suis seule, quelques soirées et la demande est pressante !!!) Mais je me justifie, ce qui n'a pas lieu d'être.

Merci pour vos conseils de lisibilité, je vais en tenir compte.
Pour ce qui est d'une mauvaise conception, j'ai compris la philosophie. Mais ne suis pas sûre de savoir faire la distinction lors de mon travail.
Mais, puisque c'est là que peut se nicher le ralentissement, je vais regarder cela de plus près.

Encore un grand merci.
Est-il judicieux de laisser ce code sur le forum?
Peut-on le faire disparaître?

Claude