Vous êtes nouveau sur Developpez.com ? Créez votre compte ou connectez-vous afin de pouvoir participer !

Vous devez avoir un compte Developpez.com et être connecté pour pouvoir participer aux discussions.

Vous n'avez pas encore de compte Developpez.com ? Créez-en un en quelques instants, c'est entièrement gratuit !

Si vous disposez déjà d'un compte et qu'il est bien activé, connectez-vous à l'aide du formulaire ci-dessous.

Identifiez-vous
Identifiant
Mot de passe
Mot de passe oublié ?
Créer un compte

L'inscription est gratuite et ne vous prendra que quelques instants !

Je m'inscris !

Choses à ne surtout pas faire dans vos applications
Participez à l'élaboration d'une liste de pratiques à éviter

Le , par Paul TOTH

6PARTAGES

11  0 
Choses à ne pas faire
Je vous propose de dresser une liste de choses à ne surtout pas faire dans vos applications

Bonjour,

Je vous propose de lancer dans ce fil, une liste de choses à ne surtout pas faire dans vos applications.

Je suis sur une maintenance applicative d'un code qui a débuté il y a 10 ans, il est complexe et contient des tas de choses à ne pas faire (CANPF).

La CANPF du jour :

Sur une fiche, vous placez un code d'initialisation sur, au choix, OnCreate, OnShow, OnActivate...

Plus tard, vous avez besoin de lancer ce code d'initialisation dans un contexte spécifique...exemple la fiche concerne un client et vous avez un bouton qui permet de passer sur un autre client, du coup il faut reinitialiser la fiche avec ce nouveau client. Et là vous faites appel à FormShow(nil) par exemple:

Code : Sélectionner tout
1
2
3
4
5
6
7
8
9
10
11
12
13
procedure TForm1.FormShow(Sender: TObject);
begin
  if Condition1 then
    ListBox1.ItemIndex := 12;
  if Condition2 then
    Label1.Visible := False;
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
  FormShow(nil);
end;
et puis finalement vous remarquez que vous ne devez pas faire exactement le même traitement, la condition2 doit être fausse...deux solutions aussi moches l'une que l'autre :

Code : Sélectionner tout
1
2
3
4
5
6
7
8
9
10
procedure TForm1.Button1Click(Sender: TObject);
var
  OldCondition2: Boolean;
begin
  OldCondition2 := Condition2;
  Condition2 := False;
  FormShow(nil);
  Condition2 := OldCondition2;
end;
Code : Sélectionner tout
1
2
3
4
5
6
7
8
procedure TForm1.FormShow(Sender: TObject);
begin
  if Condition1 then
    ListBox1.ItemIndex := 12;
  if Condition2 and (Sender <> nil) then
    Label1.Visible := False;
end;
à force de rustine de ce type vous finissez par avoir un code totalement illisible et incompréhensible.

donc ne faites JAMAIS ça ! la méthode FormShow doit être déclenchée par l'affichage de la fiche et point c'est tout !!!!

la bonne solution étant celle-ci par exemple :
Code : Sélectionner tout
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
procedure TForm1.FormShow(Sender: TObject);
begin
  InitialiserListBox1;
  if Condition2 then
    Label1.Visible := False;
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
  InitialiserListBox1;
end;

procedure TForm1.InitialiserListBox1;
begin
  if Condition1 then
    ListBox1.ItemIndex := 12;
end;

Une erreur dans cette actualité ? Signalez-le nous !

Avatar de Ph. B.
Expert confirmé https://www.developpez.com
Le 14/04/2015 à 12:01
Bonjour,

La CANPFPaul TOTH) que je décris fait suite à une erreur que l'on rencontre également trop souvent lors de la maintenance applicative et qui revient régulièrement dans les codes associés aux discussions dans les forums.

Il s'agit de l'emploi d'une variable instance de classe en lieu et place de Self dans une méthode de classe.

Je m'appuierai dans l'exemple qui suit sur la classe TForm, mais cela peut s'appliquer à toutes les classes utilisées que ce soient celles dérivées de préexistantes (TForm, TDataModule, etc) ou celles créées de toute pièce par vos soins.

Exemple :
Code : Sélectionner tout
1
2
3
4
5
6
7
8
9
procedure TForm1.Button1Click(Sender: TObject);
begin
  // 
  Form1.Button1.Enabled := False;
  // 
  Form1.MonTraitement;
  //
  Form1.Button1.Enabled := True;
end;
La méthode MonTraitement va réaliser diverses opérations dépendant de données que vous aurez préalablement renseignées sur cette même fiche (zones de saisie, case à cocher, bouton radio) ou sélectionnées (lignes d'une grille TDBGrid).

Vous allez objecter : Ce code compile sans erreur et ne déclenche pas d'erreur à l'exécution !

Soit, mais supposons qu’ultérieurement vous instanciez dans votre application une deuxième fiche de Type TForm1 dont la variable d'instance sera nommée Form1Bis. Que va-t-il se passer à l'exécution lors d'un clic sur le bouton Button1 de l'instance Form1Bis ?
  1. Si votre fiche Form1 n'est pas instanciée ou ne l'est plus, vous aurez un beau message d'erreur : une violation d'accès.
    Moindre mal dirais-je, le problème devrait être vite identifié et corrigé !
  2. Si votre fiche Form1 est instanciée, le traitement va être réalisé très probablement sans erreur, mais avec des données et paramètres qui ne sont pas ceux que vous aviez préalablement saisis ou choisis !
    Les conséquences pourraient être lourdes (données comptables, médicales, etc) et surtout n'apparaitre que bien plus tard !
    L'identification du bug sera plus longue et la correction ne se limitera pas simplement à l'applicatif ni même à la (ou aux) base(s) de données impactée(s)...


Ce qu'il aurait été nécessaire d'écrire :
Code : Sélectionner tout
1
2
3
4
5
6
7
8
9
procedure TForm1.Button1Click(Sender: TObject);
begin
  // 
  Self.Button1.Enabled := False;
  //
  Self.MonTraitement;
  //
  Self.Button1.Enabled := True;
end;
Dernière remarque, le terme Self peut être omis car implicite dans une méthode de classe. Mais, attention, son absence peut amener à certaines ambigüités dans le code que le compilateur ne lèvera pas de la manière dont vous l'auriez souhaitée, en particulier lors de l'utilisation du mot clé with, mais ceci pourrait faire l'objet d'une autre CANPF .
8  0 
Avatar de Paul TOTH
Expert éminent sénior https://www.developpez.com
Le 14/04/2015 à 14:02
Merci Ph.B.

ma CANPF du jour...j'en reviens pas que quelqu'un a écrit cela, car c'est bien un exemple réel:

Code : Sélectionner tout
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
procedure TForm1.Edit1KeyDown(Sender: TObject; var Key: Word;
  Shift: TShiftState);
begin
  if (Key = VK_F4) and (Edit1.Focused) then // événement lié à plusieurs zone TEdit
    TForm2.Create(Self); // liste de choix pour le champ Edit1
end;

procedure TForm1.Button1Click(Sender: TObject);
var
  Key: Word;
begin
  Edit1.SetFocus();
  Key := VK_F4;
  Edit1KeyDown(nil, Key); // ouch !
end;
le code à écrire sera évidemment
Code : Sélectionner tout
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
procedure TForm1.Edit1KeyDown(Sender: TObject; var Key: Word;
  Shift: TShiftState);
begin
  if (Sender = Edit1) and (Key = VK_F4) then
    ListeDeChoix();
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
  ListeDeChoix();
end;

procedure TForm1.ListeDeChoix();
begin
  TForm2.Create(Self);
end;
NB: j'utilise délibérément des noms génériques Button1, Form1...pour éviter d'avoir à préciser que ce sont des fiches, des boutons, mais un code réel devrait évidemment nommer explicitement les composants
4  0 
Avatar de e-ric
Membre expert https://www.developpez.com
Le 18/04/2015 à 12:10
Salut à tous,

J'apporte ma petite contribution (en Free Pascal ). Je vais parler d'une erreur commune souvent liée à une mauvaise appréciation des nombres flottants, le problème n'est pas lié à un langage en particulier.

Dans les calculs itératifs sur les réels, on est souvent amené à comparer 2 réels pour en vérifier la terminaison, l'erreur réside dans leur comparaison directe, en négligeant l'imprécision intrinsèque des flottants, on peut ainsi aboutir à des boucles infinies.

Pour illustrer ceci, je propose un calcul itératif simple pour laquelle la convergence est connue, il s'agit de la somme d'une suite géométrique de raison q, 0<q<1 (je vous renvoie à vos livres de maths préférés ). La limite est facile à calculer, elle vaut q/(q-1). Le calcul itératif ne sert pas à grand chose puisqu'on connaît le résultat général mais il permet de mettre en évidence le problème.

Le code est le suivant (programme complet):
Code : Sélectionner tout
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
#!/usr/bin/instantfpc
// supprimer la 1ère ligne pour Delphi
program Flottants;

{$mode objfpc}{$H+} // Supprimer cette ligne pour Delphi

uses
  Classes, SysUtils;

const
  Values : array[1..17] of Extended = ( 2,  3,  5,  7, 11, 13, 17, 19,
                                       23, 29, 31, 37, 41, 43, 47, 53, 
                                      101);
  MaxIter = 100000;
  LimiteAtteinte : array[boolean] of String = ('Non', 'Oui');

function ANePasFaire(const N: Extended): Integer;
var
  P,  // le terme de la suite
  S,  // somme des termes
  L: Extended; // Valeur de la limite
begin
  if N = 0 then raise EDivByZero.Create('Erreur: Argument nul.'); // pas le choix, ici
  S := 1;
  P := 1;
  L := N / (N - 1);
  Result := 1; // Compteur d'itération
  // Pb sur (S<>L)!!!, heureusement, les itérations sont contrôlées
  while (S <> L) and (Result < MaxIter) do
  begin
    P := P / N;
    S := S + P;
    Inc(Result);
  end;
End;

// On ne compare pas entre eux des nombres flottants
function PlusCorrect(const N: Extended): Integer;
const
  Precision : Extended = 1E-9;
var
  P,  // le terme de la suite
  S,  // somme des termes
  L: Extended; // Valeur de la limite
begin
  if N = 0 then raise EDivByZero.Create('Erreur: Argument nul.'); // pas le choix, ici
  S := 1;
  P := 1;
  L := N / (N - 1);
  Result := 1; // Compteur d'itération
  // On compare la différence à une précision donnée
  while (Abs(S - L) >= Precision) and (Result < MaxIter) do
  begin
    P := P / N;
    S := S + P;
    Inc(Result);
  end;
End;

var
  v: Extended;
  nIter: Integer;
begin
  WriteLn('** Début de test *********');
  WriteLn('  ** Code fautif *********');
  for v in Values do  // instruction non vérifiée dans Delphi
  begin
    nIter := ANePasFaire(v);
    WriteLn(Format('    %6d itérations pour %3.0f, limite atteinte: %s', [ nIter, v, LimiteAtteinte[nIter<>MaxIter]]));
  end;
  WriteLn('  ** Code raisonnable ****');
  for v in Values do // instruction non vérifiee dans Delphi
  begin
    nIter := PlusCorrect(v);
    WriteLn(Format('    %6d itérations pour %3.0f, limite atteinte: %s (avec la précision voulue)', [ nIter, v, LimiteAtteinte[nIter<>MaxIter]]));
  end;
  WriteLn('** Fin de test ***********');
  ReadLn;
end.
Les résultats obtenus sont les suivants:
Code : Sélectionner tout
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
** Début de test *********
  ** Code fautif *********
        65 itérations pour   2, limite atteinte: Oui
    100000 itérations pour   3, limite atteinte: Non
        27 itérations pour   5, limite atteinte: Oui
    100000 itérations pour   7, limite atteinte: Non
        19 itérations pour  11, limite atteinte: Oui
    100000 itérations pour  13, limite atteinte: Non
    100000 itérations pour  17, limite atteinte: Non
    100000 itérations pour  19, limite atteinte: Non
        15 itérations pour  23, limite atteinte: Oui
        14 itérations pour  29, limite atteinte: Oui
        13 itérations pour  31, limite atteinte: Oui
        13 itérations pour  37, limite atteinte: Oui
        12 itérations pour  41, limite atteinte: Oui
    100000 itérations pour  43, limite atteinte: Non
    100000 itérations pour  47, limite atteinte: Non
    100000 itérations pour  53, limite atteinte: Non
    100000 itérations pour 101, limite atteinte: Non
  ** Code raisonnable ****
        31 itérations pour   2, limite atteinte: Oui (avec la précision voulue)
        20 itérations pour   3, limite atteinte: Oui (avec la précision voulue)
        14 itérations pour   5, limite atteinte: Oui (avec la précision voulue)
        11 itérations pour   7, limite atteinte: Oui (avec la précision voulue)
         9 itérations pour  11, limite atteinte: Oui (avec la précision voulue)
         9 itérations pour  13, limite atteinte: Oui (avec la précision voulue)
         8 itérations pour  17, limite atteinte: Oui (avec la précision voulue)
         8 itérations pour  19, limite atteinte: Oui (avec la précision voulue)
         7 itérations pour  23, limite atteinte: Oui (avec la précision voulue)
         7 itérations pour  29, limite atteinte: Oui (avec la précision voulue)
         7 itérations pour  31, limite atteinte: Oui (avec la précision voulue)
         6 itérations pour  37, limite atteinte: Oui (avec la précision voulue)
         6 itérations pour  41, limite atteinte: Oui (avec la précision voulue)
         6 itérations pour  43, limite atteinte: Oui (avec la précision voulue)
         6 itérations pour  47, limite atteinte: Oui (avec la précision voulue)
         6 itérations pour  53, limite atteinte: Oui (avec la précision voulue)
         5 itérations pour 101, limite atteinte: Oui (avec la précision voulue)
** Fin de test ***********
On constate que :
- pour le cas CANPF (fonction ANePasFaire) la limite n'est pas toujours atteinte avec un risque de boucle infinie.
- Cette limite peut être atteinte avec une précision raisonnable et un nombre d'itérations moindre en codant de manière adaptée (fonction PlusCorrect)
- La différence de code est minime, la mauvaise implémentation est donc difficilement justifiable.

Moralité : Eviter les comparaisons entre des nombres flottants issus de calculs (je ne parle pas des constantes).
4  0 
Avatar de Paul TOTH
Expert éminent sénior https://www.developpez.com
Le 21/03/2016 à 14:18
retour sur une CANPF

j'expliquais précédemment de ne pas faire appel à une méthode de type "Button1Click(Self)" qui n'a pas de signification propre... voici un exemple concret de régression causée par ce genre de pratique.

Soit un TPageControl contenant des pages avec des événements OnShow qui sont utilisés pour initialiser leur contenu.

le code en question utilisait TabSheet1OnShow(nil) pour forcer l'actualisation de certains champs... sauf qu'au cours du développement, les champs en question ont été déplacés vers une nouvelle page car la précédente devenait trop chargée... partout où le code avait besoin d'actualiser les contrôles, il appelait TabSheet1OnShow() dont le code n'était plus adapté.

Code : Sélectionner tout
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
procedure TForm1.TabSheet1Show(Sender: TObject);
begin
  Edit1.Text := 'Valeur par défaut';
//  Edit2.Text := 'Valeur par défaut'; <-- déplacé dans TabSheet2
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
  TabSheet1Show(nil); // recharge les valeurs par défaut de Edit1 et Edit2
end;

procedure TForm1.TabSheet2Show(Sender: TObject);
begin
  Edit2.Text := 'Valeur par défaut';
end;
dans cet exemple Edit2 n'est plus mis à jour par Button1.

NB: évidement dans l'exemple réel, les méthodes sont plus complexes et TabSheet1Show() est appelé à plusieurs reprises dans différents endroits du code...et tout cela sans commentaires
3  0 
Avatar de Paul TOTH
Expert éminent sénior https://www.developpez.com
Le 22/03/2016 à 23:29
Citation Envoyé par anapurna Voir le message
Salut

je reviens sur un code proposé ce qui m’énerve le plus c'est la non exploitation du sender dans les Evénement des composants

je reviens donc sur un code fournis
voici un exemple concret

j'aurais cent fois préféré cette solution

Code : Sélectionner tout
1
2
3
4
5
6
7
8
9
procedure TForm1.Button1Click(Sender: TObject);
begin
  // 
  (Sender as Tbutton).Enabled := False;
  //
  MonTraitement;
  //
  (Sender as Tbutton).Enabled := True;
end;
cela facilite la maintenance du code quand vous modifiez le nom du composant on est pas obligé de revenir dans tout le code ou le nom de celui-ci est écrit en dure
de plus cela permet d'affecter un même événement a plusieurs composant de même type ayant la même action a généré

mes 2 cents
non en fait je n'aime pas cet exemple

tu peux très couramment associer le même évènement à un MenuItem et un Button, si tu présuppose que le Sender est un TButton, tu ne peux plus le faire.

d'autre par, je préfère cette approche:

Code : Sélectionner tout
1
2
3
4
5
6
7
8
9
10
11
procedure TForm1.Button1Click(Sender: TObject);
begin
  DisableSave();
end;

procedure TForm1.DisableSave();
begin
  btSave.Enabled := False;
  mnSave.Enabled := False;
  mnSaveAs.Enabled := False;
end;
ou à la rigueur

Code : Sélectionner tout
1
2
3
4
5
6
procedure TForm1.DisableSave(Sender: TObject);
begin
  btSave.Enabled := False;
  mnSave.Enabled := False;
  mnSaveAs.Enabled := False;
end;
comme ça tu peux indiquer directement dans Button1 que OnClick fait un DisableSave...mais tu es obligé de revenir à la première écriture dès que tu as plusieurs actions pas forcément déclenchée en même temps:

Code : Sélectionner tout
1
2
3
4
5
6
7
8
9
10
11
procedure TForm1.Button1Click(Sender: TObject);
begin
  SaveDocument();
  DisableSave();
end;

procedure TForm1.Button2Click(Sender: TObject);
begin
  NewDocument();
  DisableSave();
end;
3  0 
Avatar de Paul TOTH
Expert éminent sénior https://www.developpez.com
Le 14/04/2015 à 19:43
dans ma première réponse je précisais aussi que si un bouton se déclenche sur MouseUp alors qu'un raccourci clavier se déclenche sur KeyDown c'est qu'en faisant sortir la souris du bouton il est possible d'annuler le clic...Alors qu'il est impossible d'annuler la pression sur une touche
2  0 
Avatar de e-ric
Membre expert https://www.developpez.com
Le 15/04/2015 à 12:31
Salut à tous

L'idée des CANPF est pertinente et pourra éviter aux débutants (ou aux séniors distraits ou fatigués) de faire des sotises.
Cependant, est-ce que cela ne pourrait pas plutôt figurer dans le wiki, ce serait plus pratique. Le fil de discussion risque en effet de devenir difficile à exploiter à la longue.

Cdlt
2  0 
Avatar de Paul TOTH
Expert éminent sénior https://www.developpez.com
Le 21/03/2016 à 20:42
oui ton code est trop long, mais on voit bien qu'il pourrait assez facilement être découpé en plusieurs étapes.

ceci dit je constate que tu fais trop de C++ quand tu écris "ASessionID: Integer; ASessionJour: Integer; ASessionVague: Integer; ASessionDropSide: Integer", c'est un de mes grands plaisir quand je traduis du code C++ en Delphi que d'écrire "ASessionID, ASessionJour, ASessionVague, ASessionDropSide: Integer"

et non je ne suis pas un puriste du 80x25, mais quand j'ai plus de 2 niveaux d'indentation ou que le "if then else" mais fait défiler de plusieurs écrans, je regarde de suite si je ne peux pas séparer les choses
2  0 
Avatar de foetus
Expert éminent https://www.developpez.com
Le 22/03/2016 à 10:35
Citation Envoyé par Paul TOTH  Voir le message
oui moi je ne suis pas très fan, tout comme les énumérés préfixés du nom du type...ça me fait un peu trop penser à Java

Non cela permet de repérer facilement ce que c'est (puisque c'est une valeur prédéfinie/ spéciale parmi X) et surtout ne pas avoir de collisions de nom/ de doublons

Citation Envoyé par Paul TOTH  Voir le message
oui je sais, mais il manque le bouton qui va bien dans la barre d'outils déjà les [] c'est pas génial à taper, mais je me plante souvent en tapant codeinlne codinline codeinline...alors que les guillemets c'est super facile

Il y a la version simplifiée avec juste un c

Par contre, cette balise mange la fin de ligne . Il faut impérativement un caractère en plus derrière (comme un point)

Et il me semble que tu peux spécifier le langage
Édit 1: ici sur cette page
Édit 2: aussi sur cette page

Test:
Code C : Sélectionner tout
void  test(std::vector<int>);
.
Code C++ : Sélectionner tout
void  test(std::vector<int>);
.
2  0 
Avatar de GoustiFruit
Membre éclairé https://www.developpez.com
Le 22/03/2016 à 20:13
Citation Envoyé par anapurna Voir le message
salut

dans le cas de sa propre méthode tu n'a pas besoin de tester si c'est un bouton vu que l’émetteur de l’événement est le bouton lui même
c'est quand tu bidouille avec les événement qu'il est dans ce cas obligatoire de se proteger
Oui... mais non ! On peut toujours avoir un appel à AnyButtonClick depuis n'importe où dans le code ! Et donc lui passer en paramètre ce que l'on veut.
2  0